Message ID | 1691460215-45383-1-git-send-email-quic_qianyu@quicinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c44e:0:b0:3f2:4152:657d with SMTP id w14csp1844654vqr; Mon, 7 Aug 2023 20:01:44 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHKtY72C3Mjp5h9Q6cSUgo/qt3jYMre3MyoTfsGPQXF7QnBkfbl1y8p39dscJfidteFjzNo X-Received: by 2002:a05:6a20:3259:b0:13d:c70d:de62 with SMTP id hm25-20020a056a20325900b0013dc70dde62mr9612343pzc.22.1691463703870; Mon, 07 Aug 2023 20:01:43 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691463703; cv=none; d=google.com; s=arc-20160816; b=z+1s9IUHrRjUSf/BGgOMFFrw99w7ou7u/4v/ClvWNobFdM10Nt9petpTU8Xf4vOOCT e/+2PyBQFPiAexizAQ6gnA3QH4oTlGSPKwXOA7w7ViL3TJpv1VMJM/Ts0FmBdgkVmyZS Gn4jgm9j5CgL1k1vtxTMPt8BrvxHIWUAtlE0NgtAkE7HXGe4BG9S0CNwf627q+2YNJKE NHSXtODZbZ4upk5ccpeKuZscsmGbhLjrmgV8hGd2BTgMsHT2vnJzQEANmMyWlGDUHKTe N1DKVmEBadT9Ry/GGZQrm0n8vjgCXCySznCFuTn93C821X28NZQQJykSIa2h1bK5MJV1 m6vQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:message-id:date:subject:cc:to:from :dkim-signature; bh=LxDossKSwlJIU0cudzztIGuVUeJBaXR3bfGTOOyB8C8=; fh=XGjc6DN2TUSLPbAXROPgB5K42GJdzOr7S+StNr3KTvg=; b=HapPvj3ftObzA/dRFIHZJXGM8q57BH0QH1JXmymL4jC/XqRLoqu1G8KupabqChH7Ui o/nPcKcXdJfBB2EXx+hRn0VFWishZ09I/rySpEw7ezExai0keZZ66CWWciBXRhm5WjTU EcISpsH0qfF0LtoZ69cpvuiQc+V/OzB24+0+2OB9azLOQD/PpN5nPIcNnEbJIpwiczIQ n+FmTUqQeu384WQ2dFl4B3TgTOl0dRJf2usD5a/ZT13LgqbUdvXMLhKefTAJSGC0Kamz kDYGta4T91mBIki9iFvRmr3cHLFFNzunfzUMvLCR39O2wuxPPk78wEWdWjgqYHCPAh0U Anrg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=W3ZPs9zB; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id a72-20020a63904b000000b00564bead481dsi5555156pge.40.2023.08.07.20.01.30; Mon, 07 Aug 2023 20:01:43 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=W3ZPs9zB; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229851AbjHHCD5 (ORCPT <rfc822;aaronkmseo@gmail.com> + 99 others); Mon, 7 Aug 2023 22:03:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40390 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229587AbjHHCDz (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 7 Aug 2023 22:03:55 -0400 Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E0A87138; Mon, 7 Aug 2023 19:03:53 -0700 (PDT) Received: from pps.filterd (m0279873.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3781xsnk010541; Tue, 8 Aug 2023 02:03:41 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=from : to : cc : subject : date : message-id; s=qcppdkim1; bh=LxDossKSwlJIU0cudzztIGuVUeJBaXR3bfGTOOyB8C8=; b=W3ZPs9zB+nsBQ4Ehh7FsUtnZa7q7OhCdoK7Sl1PpphZqKIblh+T2EmbxCYPwsJnnz/6O ln0aaJpXq2Ofp5x0JuOeKqIctH33u21s/ostpE+quD+bSAIvldeNWUNei7eDC484bFcs /kdPQQvq8i+1tf6BfmxtHJNAb8FsnIBqTg9iCRADexynY5rY1SVmbLQyBy7kqUy3kCvg iyI//ldp71pkwgOMnyIMZFHnQy5TGQL5Dqf+g4MAcSnFN+8dfay/P3UuDRL4OmdolbXV a1Bskx8cS5lhGm6zZxFFE+0771V78xOLUtjgCIQLRHdLbrkcvhkCmGL8fNQi0EQGArix Nw== Received: from aptaippmta01.qualcomm.com (tpe-colo-wan-fw-bordernet.qualcomm.com [103.229.16.4]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3sb7bb8g1b-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 08 Aug 2023 02:03:41 +0000 Received: from pps.filterd (APTAIPPMTA01.qualcomm.com [127.0.0.1]) by APTAIPPMTA01.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTP id 37823clT018674; Tue, 8 Aug 2023 02:03:38 GMT Received: from pps.reinject (localhost [127.0.0.1]) by APTAIPPMTA01.qualcomm.com (PPS) with ESMTPS id 3s9fgkmtt7-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 08 Aug 2023 02:03:38 +0000 Received: from APTAIPPMTA01.qualcomm.com (APTAIPPMTA01.qualcomm.com [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 37823bh7018518; Tue, 8 Aug 2023 02:03:37 GMT Received: from cbsp-sh-gv.qualcomm.com (CBSP-SH-gv.ap.qualcomm.com [10.231.249.68]) by APTAIPPMTA01.qualcomm.com (PPS) with ESMTP id 37823bmw018516; Tue, 08 Aug 2023 02:03:37 +0000 Received: by cbsp-sh-gv.qualcomm.com (Postfix, from userid 4098150) id E91344C6E; Tue, 8 Aug 2023 10:03:36 +0800 (CST) From: Qiang Yu <quic_qianyu@quicinc.com> To: mani@kernel.org, quic_jhugo@quicinc.com Cc: mhi@lists.linux.dev, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, quic_cang@quicinc.com, quic_mrana@quicinc.com, Qiang Yu <quic_qianyu@quicinc.com> Subject: [PATCH] bus: mhi: host: pci_generic: Add SDX75 based modem support Date: Tue, 8 Aug 2023 10:03:35 +0800 Message-Id: <1691460215-45383-1-git-send-email-quic_qianyu@quicinc.com> X-Mailer: git-send-email 2.7.4 X-QCInternal: smtphost X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: gb2SbNNLbgcahB7S_gX2R7j1Iwh48w7T X-Proofpoint-ORIG-GUID: gb2SbNNLbgcahB7S_gX2R7j1Iwh48w7T X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.267,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26 definitions=2023-08-07_28,2023-08-03_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 priorityscore=1501 mlxscore=0 adultscore=0 bulkscore=0 malwarescore=0 impostorscore=0 spamscore=0 phishscore=0 mlxlogscore=999 suspectscore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2306200000 definitions=main-2308080016 X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,SPF_HELO_NONE, SPF_NONE,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773628244466462667 X-GMAIL-MSGID: 1773628244466462667 |
Series |
bus: mhi: host: pci_generic: Add SDX75 based modem support
|
|
Commit Message
Qiang Yu
Aug. 8, 2023, 2:03 a.m. UTC
Add generic info for SDX75 based modems. SDX75 takes longer than expected
(default, 8 seconds) to set ready after reboot. Hence add optional ready
timeout parameter to wait enough for device ready as part of power up
sequence.
Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
---
drivers/bus/mhi/host/init.c | 1 +
drivers/bus/mhi/host/main.c | 7 ++++++-
drivers/bus/mhi/host/pci_generic.c | 22 ++++++++++++++++++++++
drivers/bus/mhi/host/pm.c | 6 +++++-
include/linux/mhi.h | 4 ++++
5 files changed, 38 insertions(+), 2 deletions(-)
Comments
On Tue, Aug 08, 2023 at 10:03:35AM +0800, Qiang Yu wrote: > Add generic info for SDX75 based modems. SDX75 takes longer than expected > (default, 8 seconds) to set ready after reboot. Hence add optional ready > timeout parameter to wait enough for device ready as part of power up > sequence. > > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> > --- > drivers/bus/mhi/host/init.c | 1 + > drivers/bus/mhi/host/main.c | 7 ++++++- > drivers/bus/mhi/host/pci_generic.c | 22 ++++++++++++++++++++++ > drivers/bus/mhi/host/pm.c | 6 +++++- > include/linux/mhi.h | 4 ++++ > 5 files changed, 38 insertions(+), 2 deletions(-) > > diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c > index f78aefd..65ceac1 100644 > --- a/drivers/bus/mhi/host/init.c > +++ b/drivers/bus/mhi/host/init.c > @@ -881,6 +881,7 @@ static int parse_config(struct mhi_controller *mhi_cntrl, > if (!mhi_cntrl->timeout_ms) > mhi_cntrl->timeout_ms = MHI_TIMEOUT_MS; > > + mhi_cntrl->ready_timeout_ms = config->ready_timeout_ms; > mhi_cntrl->bounce_buf = config->use_bounce_buf; > mhi_cntrl->buffer_len = config->buf_len; > if (!mhi_cntrl->buffer_len) > diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c > index 74a7543..8590926 100644 > --- a/drivers/bus/mhi/host/main.c > +++ b/drivers/bus/mhi/host/main.c > @@ -43,8 +43,13 @@ int __must_check mhi_poll_reg_field(struct mhi_controller *mhi_cntrl, > u32 mask, u32 val, u32 delayus) > { > int ret; > - u32 out, retry = (mhi_cntrl->timeout_ms * 1000) / delayus; > + u32 out, retry; > + u32 timeout_ms = mhi_cntrl->timeout_ms; > > + if (mhi_cntrl->ready_timeout_ms && mask == MHISTATUS_READY_MASK) > + timeout_ms = mhi_cntrl->ready_timeout_ms; Instead of handling the timeout inside mhi_poll_reg_field(), you should pass the appropriate timeout value to this function. > + > + retry = (timeout_ms * 1000) / delayus; > while (retry--) { > ret = mhi_read_reg_field(mhi_cntrl, base, offset, mask, &out); > if (ret) > diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c > index fcd80bc..9c601f0 100644 > --- a/drivers/bus/mhi/host/pci_generic.c > +++ b/drivers/bus/mhi/host/pci_generic.c > @@ -269,6 +269,16 @@ static struct mhi_event_config modem_qcom_v1_mhi_events[] = { > MHI_EVENT_CONFIG_HW_DATA(5, 2048, 101) > }; > > +static const struct mhi_controller_config modem_qcom_v2_mhiv_config = { > + .max_channels = 128, > + .timeout_ms = 8000, > + .ready_timeout_ms = 50000, > + .num_channels = ARRAY_SIZE(modem_qcom_v1_mhi_channels), > + .ch_cfg = modem_qcom_v1_mhi_channels, > + .num_events = ARRAY_SIZE(modem_qcom_v1_mhi_events), > + .event_cfg = modem_qcom_v1_mhi_events, > +}; > + > static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { > .max_channels = 128, > .timeout_ms = 8000, > @@ -278,6 +288,16 @@ static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { > .event_cfg = modem_qcom_v1_mhi_events, > }; > > +static const struct mhi_pci_dev_info mhi_qcom_sdx75_info = { > + .name = "qcom-sdx75m", > + .fw = "qcom/sdx75m/xbl.elf", > + .edl = "qcom/sdx75m/edl.mbn", > + .config = &modem_qcom_v2_mhiv_config, > + .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > + .dma_data_width = 32, > + .sideband_wake = false, > +}; > + > static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = { > .name = "qcom-sdx65m", > .fw = "qcom/sdx65m/xbl.elf", > @@ -597,6 +617,8 @@ static const struct pci_device_id mhi_pci_id_table[] = { > .driver_data = (kernel_ulong_t) &mhi_telit_fn990_info }, > { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0308), > .driver_data = (kernel_ulong_t) &mhi_qcom_sdx65_info }, > + { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0309), > + .driver_data = (kernel_ulong_t) &mhi_qcom_sdx75_info }, > { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1001), /* EM120R-GL (sdx24) */ > .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info }, > { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1002), /* EM160R-GL (sdx24) */ > diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c > index 8a4362d..6f049e0 100644 > --- a/drivers/bus/mhi/host/pm.c > +++ b/drivers/bus/mhi/host/pm.c > @@ -1202,14 +1202,18 @@ EXPORT_SYMBOL_GPL(mhi_power_down); > int mhi_sync_power_up(struct mhi_controller *mhi_cntrl) > { > int ret = mhi_async_power_up(mhi_cntrl); > + u32 timeout_ms; > > if (ret) > return ret; > > + /* Some devices need more time to set ready during power up */ > + timeout_ms = mhi_cntrl->ready_timeout_ms ? > + mhi_cntrl->ready_timeout_ms : mhi_cntrl->timeout_ms; Since you are using this extended timeout value in a couple of places (not just for checking READY_STATE), it is better to use the existing "timeout_ms" parameter. - Mani > wait_event_timeout(mhi_cntrl->state_event, > MHI_IN_MISSION_MODE(mhi_cntrl->ee) || > MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state), > - msecs_to_jiffies(mhi_cntrl->timeout_ms)); > + msecs_to_jiffies(timeout_ms)); > > ret = (MHI_IN_MISSION_MODE(mhi_cntrl->ee)) ? 0 : -ETIMEDOUT; > if (ret) > diff --git a/include/linux/mhi.h b/include/linux/mhi.h > index f6de4b6..a43e5f8 100644 > --- a/include/linux/mhi.h > +++ b/include/linux/mhi.h > @@ -266,6 +266,7 @@ struct mhi_event_config { > * struct mhi_controller_config - Root MHI controller configuration > * @max_channels: Maximum number of channels supported > * @timeout_ms: Timeout value for operations. 0 means use default > + * @ready_timeout_ms: Timeout value for waiting device to be ready (optional) > * @buf_len: Size of automatically allocated buffers. 0 means use default > * @num_channels: Number of channels defined in @ch_cfg > * @ch_cfg: Array of defined channels > @@ -277,6 +278,7 @@ struct mhi_event_config { > struct mhi_controller_config { > u32 max_channels; > u32 timeout_ms; > + u32 ready_timeout_ms; > u32 buf_len; > u32 num_channels; > const struct mhi_channel_config *ch_cfg; > @@ -326,6 +328,7 @@ struct mhi_controller_config { > * @pm_mutex: Mutex for suspend/resume operation > * @pm_lock: Lock for protecting MHI power management state > * @timeout_ms: Timeout in ms for state transitions > + * @ready_timeout_ms: Timeout in ms for waiting device to be ready (optional) > * @pm_state: MHI power management state > * @db_access: DB access states > * @ee: MHI device execution environment > @@ -413,6 +416,7 @@ struct mhi_controller { > struct mutex pm_mutex; > rwlock_t pm_lock; > u32 timeout_ms; > + u32 ready_timeout_ms; > u32 pm_state; > u32 db_access; > enum mhi_ee_type ee; > -- > 2.7.4 >
On 8/8/2023 3:51 PM, Manivannan Sadhasivam wrote: > On Tue, Aug 08, 2023 at 10:03:35AM +0800, Qiang Yu wrote: >> Add generic info for SDX75 based modems. SDX75 takes longer than expected >> (default, 8 seconds) to set ready after reboot. Hence add optional ready >> timeout parameter to wait enough for device ready as part of power up >> sequence. >> >> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> >> --- >> drivers/bus/mhi/host/init.c | 1 + >> drivers/bus/mhi/host/main.c | 7 ++++++- >> drivers/bus/mhi/host/pci_generic.c | 22 ++++++++++++++++++++++ >> drivers/bus/mhi/host/pm.c | 6 +++++- >> include/linux/mhi.h | 4 ++++ >> 5 files changed, 38 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c >> index f78aefd..65ceac1 100644 >> --- a/drivers/bus/mhi/host/init.c >> +++ b/drivers/bus/mhi/host/init.c >> @@ -881,6 +881,7 @@ static int parse_config(struct mhi_controller *mhi_cntrl, >> if (!mhi_cntrl->timeout_ms) >> mhi_cntrl->timeout_ms = MHI_TIMEOUT_MS; >> >> + mhi_cntrl->ready_timeout_ms = config->ready_timeout_ms; >> mhi_cntrl->bounce_buf = config->use_bounce_buf; >> mhi_cntrl->buffer_len = config->buf_len; >> if (!mhi_cntrl->buffer_len) >> diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c >> index 74a7543..8590926 100644 >> --- a/drivers/bus/mhi/host/main.c >> +++ b/drivers/bus/mhi/host/main.c >> @@ -43,8 +43,13 @@ int __must_check mhi_poll_reg_field(struct mhi_controller *mhi_cntrl, >> u32 mask, u32 val, u32 delayus) >> { >> int ret; >> - u32 out, retry = (mhi_cntrl->timeout_ms * 1000) / delayus; >> + u32 out, retry; >> + u32 timeout_ms = mhi_cntrl->timeout_ms; >> >> + if (mhi_cntrl->ready_timeout_ms && mask == MHISTATUS_READY_MASK) >> + timeout_ms = mhi_cntrl->ready_timeout_ms; > Instead of handling the timeout inside mhi_poll_reg_field(), you should pass the > appropriate timeout value to this function. OK, will do. > >> + >> + retry = (timeout_ms * 1000) / delayus; >> while (retry--) { >> ret = mhi_read_reg_field(mhi_cntrl, base, offset, mask, &out); >> if (ret) >> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c >> index fcd80bc..9c601f0 100644 >> --- a/drivers/bus/mhi/host/pci_generic.c >> +++ b/drivers/bus/mhi/host/pci_generic.c >> @@ -269,6 +269,16 @@ static struct mhi_event_config modem_qcom_v1_mhi_events[] = { >> MHI_EVENT_CONFIG_HW_DATA(5, 2048, 101) >> }; >> >> +static const struct mhi_controller_config modem_qcom_v2_mhiv_config = { >> + .max_channels = 128, >> + .timeout_ms = 8000, >> + .ready_timeout_ms = 50000, >> + .num_channels = ARRAY_SIZE(modem_qcom_v1_mhi_channels), >> + .ch_cfg = modem_qcom_v1_mhi_channels, >> + .num_events = ARRAY_SIZE(modem_qcom_v1_mhi_events), >> + .event_cfg = modem_qcom_v1_mhi_events, >> +}; >> + >> static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { >> .max_channels = 128, >> .timeout_ms = 8000, >> @@ -278,6 +288,16 @@ static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { >> .event_cfg = modem_qcom_v1_mhi_events, >> }; >> >> +static const struct mhi_pci_dev_info mhi_qcom_sdx75_info = { >> + .name = "qcom-sdx75m", >> + .fw = "qcom/sdx75m/xbl.elf", >> + .edl = "qcom/sdx75m/edl.mbn", >> + .config = &modem_qcom_v2_mhiv_config, >> + .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >> + .dma_data_width = 32, >> + .sideband_wake = false, >> +}; >> + >> static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = { >> .name = "qcom-sdx65m", >> .fw = "qcom/sdx65m/xbl.elf", >> @@ -597,6 +617,8 @@ static const struct pci_device_id mhi_pci_id_table[] = { >> .driver_data = (kernel_ulong_t) &mhi_telit_fn990_info }, >> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0308), >> .driver_data = (kernel_ulong_t) &mhi_qcom_sdx65_info }, >> + { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0309), >> + .driver_data = (kernel_ulong_t) &mhi_qcom_sdx75_info }, >> { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1001), /* EM120R-GL (sdx24) */ >> .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info }, >> { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1002), /* EM160R-GL (sdx24) */ >> diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c >> index 8a4362d..6f049e0 100644 >> --- a/drivers/bus/mhi/host/pm.c >> +++ b/drivers/bus/mhi/host/pm.c >> @@ -1202,14 +1202,18 @@ EXPORT_SYMBOL_GPL(mhi_power_down); >> int mhi_sync_power_up(struct mhi_controller *mhi_cntrl) >> { >> int ret = mhi_async_power_up(mhi_cntrl); >> + u32 timeout_ms; >> >> if (ret) >> return ret; >> >> + /* Some devices need more time to set ready during power up */ >> + timeout_ms = mhi_cntrl->ready_timeout_ms ? >> + mhi_cntrl->ready_timeout_ms : mhi_cntrl->timeout_ms; > Since you are using this extended timeout value in a couple of places (not just > for checking READY_STATE), it is better to use the existing "timeout_ms" > parameter. > > - Mani We use ready_timeout_ms here is because READY_STATE is polled in a workqueue, in parallel with waiting valid EE. That means we start to wait valid EE and poll ready like at same time instead of starting to wait EE after ready state. Thus the total time it takes to wait valid EE is about the time for polling ready. >> wait_event_timeout(mhi_cntrl->state_event, >> MHI_IN_MISSION_MODE(mhi_cntrl->ee) || >> MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state), >> - msecs_to_jiffies(mhi_cntrl->timeout_ms)); >> + msecs_to_jiffies(timeout_ms)); >> >> ret = (MHI_IN_MISSION_MODE(mhi_cntrl->ee)) ? 0 : -ETIMEDOUT; >> if (ret) >> diff --git a/include/linux/mhi.h b/include/linux/mhi.h >> index f6de4b6..a43e5f8 100644 >> --- a/include/linux/mhi.h >> +++ b/include/linux/mhi.h >> @@ -266,6 +266,7 @@ struct mhi_event_config { >> * struct mhi_controller_config - Root MHI controller configuration >> * @max_channels: Maximum number of channels supported >> * @timeout_ms: Timeout value for operations. 0 means use default >> + * @ready_timeout_ms: Timeout value for waiting device to be ready (optional) >> * @buf_len: Size of automatically allocated buffers. 0 means use default >> * @num_channels: Number of channels defined in @ch_cfg >> * @ch_cfg: Array of defined channels >> @@ -277,6 +278,7 @@ struct mhi_event_config { >> struct mhi_controller_config { >> u32 max_channels; >> u32 timeout_ms; >> + u32 ready_timeout_ms; >> u32 buf_len; >> u32 num_channels; >> const struct mhi_channel_config *ch_cfg; >> @@ -326,6 +328,7 @@ struct mhi_controller_config { >> * @pm_mutex: Mutex for suspend/resume operation >> * @pm_lock: Lock for protecting MHI power management state >> * @timeout_ms: Timeout in ms for state transitions >> + * @ready_timeout_ms: Timeout in ms for waiting device to be ready (optional) >> * @pm_state: MHI power management state >> * @db_access: DB access states >> * @ee: MHI device execution environment >> @@ -413,6 +416,7 @@ struct mhi_controller { >> struct mutex pm_mutex; >> rwlock_t pm_lock; >> u32 timeout_ms; >> + u32 ready_timeout_ms; >> u32 pm_state; >> u32 db_access; >> enum mhi_ee_type ee; >> -- >> 2.7.4 >>
On Tue, Aug 08, 2023 at 04:53:32PM +0800, Qiang Yu wrote: > > On 8/8/2023 3:51 PM, Manivannan Sadhasivam wrote: > > On Tue, Aug 08, 2023 at 10:03:35AM +0800, Qiang Yu wrote: > > > Add generic info for SDX75 based modems. SDX75 takes longer than expected > > > (default, 8 seconds) to set ready after reboot. Hence add optional ready > > > timeout parameter to wait enough for device ready as part of power up > > > sequence. > > > > > > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> > > > --- > > > drivers/bus/mhi/host/init.c | 1 + > > > drivers/bus/mhi/host/main.c | 7 ++++++- > > > drivers/bus/mhi/host/pci_generic.c | 22 ++++++++++++++++++++++ > > > drivers/bus/mhi/host/pm.c | 6 +++++- > > > include/linux/mhi.h | 4 ++++ > > > 5 files changed, 38 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c > > > index f78aefd..65ceac1 100644 > > > --- a/drivers/bus/mhi/host/init.c > > > +++ b/drivers/bus/mhi/host/init.c > > > @@ -881,6 +881,7 @@ static int parse_config(struct mhi_controller *mhi_cntrl, > > > if (!mhi_cntrl->timeout_ms) > > > mhi_cntrl->timeout_ms = MHI_TIMEOUT_MS; > > > + mhi_cntrl->ready_timeout_ms = config->ready_timeout_ms; > > > mhi_cntrl->bounce_buf = config->use_bounce_buf; > > > mhi_cntrl->buffer_len = config->buf_len; > > > if (!mhi_cntrl->buffer_len) > > > diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c > > > index 74a7543..8590926 100644 > > > --- a/drivers/bus/mhi/host/main.c > > > +++ b/drivers/bus/mhi/host/main.c > > > @@ -43,8 +43,13 @@ int __must_check mhi_poll_reg_field(struct mhi_controller *mhi_cntrl, > > > u32 mask, u32 val, u32 delayus) > > > { > > > int ret; > > > - u32 out, retry = (mhi_cntrl->timeout_ms * 1000) / delayus; > > > + u32 out, retry; > > > + u32 timeout_ms = mhi_cntrl->timeout_ms; > > > + if (mhi_cntrl->ready_timeout_ms && mask == MHISTATUS_READY_MASK) > > > + timeout_ms = mhi_cntrl->ready_timeout_ms; > > Instead of handling the timeout inside mhi_poll_reg_field(), you should pass the > > appropriate timeout value to this function. > OK, will do. > > > > > + > > > + retry = (timeout_ms * 1000) / delayus; > > > while (retry--) { > > > ret = mhi_read_reg_field(mhi_cntrl, base, offset, mask, &out); > > > if (ret) > > > diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c > > > index fcd80bc..9c601f0 100644 > > > --- a/drivers/bus/mhi/host/pci_generic.c > > > +++ b/drivers/bus/mhi/host/pci_generic.c > > > @@ -269,6 +269,16 @@ static struct mhi_event_config modem_qcom_v1_mhi_events[] = { > > > MHI_EVENT_CONFIG_HW_DATA(5, 2048, 101) > > > }; > > > +static const struct mhi_controller_config modem_qcom_v2_mhiv_config = { > > > + .max_channels = 128, > > > + .timeout_ms = 8000, > > > + .ready_timeout_ms = 50000, > > > + .num_channels = ARRAY_SIZE(modem_qcom_v1_mhi_channels), > > > + .ch_cfg = modem_qcom_v1_mhi_channels, > > > + .num_events = ARRAY_SIZE(modem_qcom_v1_mhi_events), > > > + .event_cfg = modem_qcom_v1_mhi_events, > > > +}; > > > + > > > static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { > > > .max_channels = 128, > > > .timeout_ms = 8000, > > > @@ -278,6 +288,16 @@ static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { > > > .event_cfg = modem_qcom_v1_mhi_events, > > > }; > > > +static const struct mhi_pci_dev_info mhi_qcom_sdx75_info = { > > > + .name = "qcom-sdx75m", > > > + .fw = "qcom/sdx75m/xbl.elf", > > > + .edl = "qcom/sdx75m/edl.mbn", > > > + .config = &modem_qcom_v2_mhiv_config, > > > + .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > > > + .dma_data_width = 32, > > > + .sideband_wake = false, > > > +}; > > > + > > > static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = { > > > .name = "qcom-sdx65m", > > > .fw = "qcom/sdx65m/xbl.elf", > > > @@ -597,6 +617,8 @@ static const struct pci_device_id mhi_pci_id_table[] = { > > > .driver_data = (kernel_ulong_t) &mhi_telit_fn990_info }, > > > { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0308), > > > .driver_data = (kernel_ulong_t) &mhi_qcom_sdx65_info }, > > > + { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0309), > > > + .driver_data = (kernel_ulong_t) &mhi_qcom_sdx75_info }, > > > { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1001), /* EM120R-GL (sdx24) */ > > > .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info }, > > > { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1002), /* EM160R-GL (sdx24) */ > > > diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c > > > index 8a4362d..6f049e0 100644 > > > --- a/drivers/bus/mhi/host/pm.c > > > +++ b/drivers/bus/mhi/host/pm.c > > > @@ -1202,14 +1202,18 @@ EXPORT_SYMBOL_GPL(mhi_power_down); > > > int mhi_sync_power_up(struct mhi_controller *mhi_cntrl) > > > { > > > int ret = mhi_async_power_up(mhi_cntrl); > > > + u32 timeout_ms; > > > if (ret) > > > return ret; > > > + /* Some devices need more time to set ready during power up */ > > > + timeout_ms = mhi_cntrl->ready_timeout_ms ? > > > + mhi_cntrl->ready_timeout_ms : mhi_cntrl->timeout_ms; > > Since you are using this extended timeout value in a couple of places (not just > > for checking READY_STATE), it is better to use the existing "timeout_ms" > > parameter. > > > > - Mani > > We use ready_timeout_ms here is because READY_STATE is polled in a > workqueue, in parallel with waiting valid EE. > > That means we start to wait valid EE and poll ready like at same time > instead of starting to wait EE after ready state. > > Thus the total time it takes to wait valid EE is about the time for polling > ready. > Yes, but why can't you still increase "timeout_ms" for SDX75 and use the same? Btw, please do not send another version while the discussion is going on for the current one. - Mani > > > wait_event_timeout(mhi_cntrl->state_event, > > > MHI_IN_MISSION_MODE(mhi_cntrl->ee) || > > > MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state), > > > - msecs_to_jiffies(mhi_cntrl->timeout_ms)); > > > + msecs_to_jiffies(timeout_ms)); > > > ret = (MHI_IN_MISSION_MODE(mhi_cntrl->ee)) ? 0 : -ETIMEDOUT; > > > if (ret) > > > diff --git a/include/linux/mhi.h b/include/linux/mhi.h > > > index f6de4b6..a43e5f8 100644 > > > --- a/include/linux/mhi.h > > > +++ b/include/linux/mhi.h > > > @@ -266,6 +266,7 @@ struct mhi_event_config { > > > * struct mhi_controller_config - Root MHI controller configuration > > > * @max_channels: Maximum number of channels supported > > > * @timeout_ms: Timeout value for operations. 0 means use default > > > + * @ready_timeout_ms: Timeout value for waiting device to be ready (optional) > > > * @buf_len: Size of automatically allocated buffers. 0 means use default > > > * @num_channels: Number of channels defined in @ch_cfg > > > * @ch_cfg: Array of defined channels > > > @@ -277,6 +278,7 @@ struct mhi_event_config { > > > struct mhi_controller_config { > > > u32 max_channels; > > > u32 timeout_ms; > > > + u32 ready_timeout_ms; > > > u32 buf_len; > > > u32 num_channels; > > > const struct mhi_channel_config *ch_cfg; > > > @@ -326,6 +328,7 @@ struct mhi_controller_config { > > > * @pm_mutex: Mutex for suspend/resume operation > > > * @pm_lock: Lock for protecting MHI power management state > > > * @timeout_ms: Timeout in ms for state transitions > > > + * @ready_timeout_ms: Timeout in ms for waiting device to be ready (optional) > > > * @pm_state: MHI power management state > > > * @db_access: DB access states > > > * @ee: MHI device execution environment > > > @@ -413,6 +416,7 @@ struct mhi_controller { > > > struct mutex pm_mutex; > > > rwlock_t pm_lock; > > > u32 timeout_ms; > > > + u32 ready_timeout_ms; > > > u32 pm_state; > > > u32 db_access; > > > enum mhi_ee_type ee; > > > -- > > > 2.7.4 > > > >
On 8/8/2023 6:59 PM, Manivannan Sadhasivam wrote: > On Tue, Aug 08, 2023 at 04:53:32PM +0800, Qiang Yu wrote: >> On 8/8/2023 3:51 PM, Manivannan Sadhasivam wrote: >>> On Tue, Aug 08, 2023 at 10:03:35AM +0800, Qiang Yu wrote: >>>> Add generic info for SDX75 based modems. SDX75 takes longer than expected >>>> (default, 8 seconds) to set ready after reboot. Hence add optional ready >>>> timeout parameter to wait enough for device ready as part of power up >>>> sequence. >>>> >>>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> >>>> --- >>>> drivers/bus/mhi/host/init.c | 1 + >>>> drivers/bus/mhi/host/main.c | 7 ++++++- >>>> drivers/bus/mhi/host/pci_generic.c | 22 ++++++++++++++++++++++ >>>> drivers/bus/mhi/host/pm.c | 6 +++++- >>>> include/linux/mhi.h | 4 ++++ >>>> 5 files changed, 38 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c >>>> index f78aefd..65ceac1 100644 >>>> --- a/drivers/bus/mhi/host/init.c >>>> +++ b/drivers/bus/mhi/host/init.c >>>> @@ -881,6 +881,7 @@ static int parse_config(struct mhi_controller *mhi_cntrl, >>>> if (!mhi_cntrl->timeout_ms) >>>> mhi_cntrl->timeout_ms = MHI_TIMEOUT_MS; >>>> + mhi_cntrl->ready_timeout_ms = config->ready_timeout_ms; >>>> mhi_cntrl->bounce_buf = config->use_bounce_buf; >>>> mhi_cntrl->buffer_len = config->buf_len; >>>> if (!mhi_cntrl->buffer_len) >>>> diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c >>>> index 74a7543..8590926 100644 >>>> --- a/drivers/bus/mhi/host/main.c >>>> +++ b/drivers/bus/mhi/host/main.c >>>> @@ -43,8 +43,13 @@ int __must_check mhi_poll_reg_field(struct mhi_controller *mhi_cntrl, >>>> u32 mask, u32 val, u32 delayus) >>>> { >>>> int ret; >>>> - u32 out, retry = (mhi_cntrl->timeout_ms * 1000) / delayus; >>>> + u32 out, retry; >>>> + u32 timeout_ms = mhi_cntrl->timeout_ms; >>>> + if (mhi_cntrl->ready_timeout_ms && mask == MHISTATUS_READY_MASK) >>>> + timeout_ms = mhi_cntrl->ready_timeout_ms; >>> Instead of handling the timeout inside mhi_poll_reg_field(), you should pass the >>> appropriate timeout value to this function. >> OK, will do. >>>> + >>>> + retry = (timeout_ms * 1000) / delayus; >>>> while (retry--) { >>>> ret = mhi_read_reg_field(mhi_cntrl, base, offset, mask, &out); >>>> if (ret) >>>> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c >>>> index fcd80bc..9c601f0 100644 >>>> --- a/drivers/bus/mhi/host/pci_generic.c >>>> +++ b/drivers/bus/mhi/host/pci_generic.c >>>> @@ -269,6 +269,16 @@ static struct mhi_event_config modem_qcom_v1_mhi_events[] = { >>>> MHI_EVENT_CONFIG_HW_DATA(5, 2048, 101) >>>> }; >>>> +static const struct mhi_controller_config modem_qcom_v2_mhiv_config = { >>>> + .max_channels = 128, >>>> + .timeout_ms = 8000, >>>> + .ready_timeout_ms = 50000, >>>> + .num_channels = ARRAY_SIZE(modem_qcom_v1_mhi_channels), >>>> + .ch_cfg = modem_qcom_v1_mhi_channels, >>>> + .num_events = ARRAY_SIZE(modem_qcom_v1_mhi_events), >>>> + .event_cfg = modem_qcom_v1_mhi_events, >>>> +}; >>>> + >>>> static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { >>>> .max_channels = 128, >>>> .timeout_ms = 8000, >>>> @@ -278,6 +288,16 @@ static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { >>>> .event_cfg = modem_qcom_v1_mhi_events, >>>> }; >>>> +static const struct mhi_pci_dev_info mhi_qcom_sdx75_info = { >>>> + .name = "qcom-sdx75m", >>>> + .fw = "qcom/sdx75m/xbl.elf", >>>> + .edl = "qcom/sdx75m/edl.mbn", >>>> + .config = &modem_qcom_v2_mhiv_config, >>>> + .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >>>> + .dma_data_width = 32, >>>> + .sideband_wake = false, >>>> +}; >>>> + >>>> static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = { >>>> .name = "qcom-sdx65m", >>>> .fw = "qcom/sdx65m/xbl.elf", >>>> @@ -597,6 +617,8 @@ static const struct pci_device_id mhi_pci_id_table[] = { >>>> .driver_data = (kernel_ulong_t) &mhi_telit_fn990_info }, >>>> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0308), >>>> .driver_data = (kernel_ulong_t) &mhi_qcom_sdx65_info }, >>>> + { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0309), >>>> + .driver_data = (kernel_ulong_t) &mhi_qcom_sdx75_info }, >>>> { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1001), /* EM120R-GL (sdx24) */ >>>> .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info }, >>>> { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1002), /* EM160R-GL (sdx24) */ >>>> diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c >>>> index 8a4362d..6f049e0 100644 >>>> --- a/drivers/bus/mhi/host/pm.c >>>> +++ b/drivers/bus/mhi/host/pm.c >>>> @@ -1202,14 +1202,18 @@ EXPORT_SYMBOL_GPL(mhi_power_down); >>>> int mhi_sync_power_up(struct mhi_controller *mhi_cntrl) >>>> { >>>> int ret = mhi_async_power_up(mhi_cntrl); >>>> + u32 timeout_ms; >>>> if (ret) >>>> return ret; >>>> + /* Some devices need more time to set ready during power up */ >>>> + timeout_ms = mhi_cntrl->ready_timeout_ms ? >>>> + mhi_cntrl->ready_timeout_ms : mhi_cntrl->timeout_ms; >>> Since you are using this extended timeout value in a couple of places (not just >>> for checking READY_STATE), it is better to use the existing "timeout_ms" >>> parameter. >>> >>> - Mani >> We use ready_timeout_ms here is because READY_STATE is polled in a >> workqueue, in parallel with waiting valid EE. >> >> That means we start to wait valid EE and poll ready like at same time >> instead of starting to wait EE after ready state. >> >> Thus the total time it takes to wait valid EE is about the time for polling >> ready. >> > Yes, but why can't you still increase "timeout_ms" for SDX75 and use the same? > > Btw, please do not send another version while the discussion is going on for the > current one. > > - Mani SDX75 only needs 50 seconds when setting ready for the first time after power on. Other state transitions is expected to wait only 8 seconds. If we use 50s for every state transition, it's OK but not friendly in some cases. For example, host is resuming from suspend, but device has already crashed and tansferred to Sahara mode when in suspended state. Thus host must wait M0 event timeout, and then reinit mhi and tranfer to SBL state in recovery process. If we set mhi_cntrl->timeout_ms=50s, we have to wait 50s to collect crash dump after seeing resume fail log. > >>>> wait_event_timeout(mhi_cntrl->state_event, >>>> MHI_IN_MISSION_MODE(mhi_cntrl->ee) || >>>> MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state), >>>> - msecs_to_jiffies(mhi_cntrl->timeout_ms)); >>>> + msecs_to_jiffies(timeout_ms)); >>>> ret = (MHI_IN_MISSION_MODE(mhi_cntrl->ee)) ? 0 : -ETIMEDOUT; >>>> if (ret) >>>> diff --git a/include/linux/mhi.h b/include/linux/mhi.h >>>> index f6de4b6..a43e5f8 100644 >>>> --- a/include/linux/mhi.h >>>> +++ b/include/linux/mhi.h >>>> @@ -266,6 +266,7 @@ struct mhi_event_config { >>>> * struct mhi_controller_config - Root MHI controller configuration >>>> * @max_channels: Maximum number of channels supported >>>> * @timeout_ms: Timeout value for operations. 0 means use default >>>> + * @ready_timeout_ms: Timeout value for waiting device to be ready (optional) >>>> * @buf_len: Size of automatically allocated buffers. 0 means use default >>>> * @num_channels: Number of channels defined in @ch_cfg >>>> * @ch_cfg: Array of defined channels >>>> @@ -277,6 +278,7 @@ struct mhi_event_config { >>>> struct mhi_controller_config { >>>> u32 max_channels; >>>> u32 timeout_ms; >>>> + u32 ready_timeout_ms; >>>> u32 buf_len; >>>> u32 num_channels; >>>> const struct mhi_channel_config *ch_cfg; >>>> @@ -326,6 +328,7 @@ struct mhi_controller_config { >>>> * @pm_mutex: Mutex for suspend/resume operation >>>> * @pm_lock: Lock for protecting MHI power management state >>>> * @timeout_ms: Timeout in ms for state transitions >>>> + * @ready_timeout_ms: Timeout in ms for waiting device to be ready (optional) >>>> * @pm_state: MHI power management state >>>> * @db_access: DB access states >>>> * @ee: MHI device execution environment >>>> @@ -413,6 +416,7 @@ struct mhi_controller { >>>> struct mutex pm_mutex; >>>> rwlock_t pm_lock; >>>> u32 timeout_ms; >>>> + u32 ready_timeout_ms; >>>> u32 pm_state; >>>> u32 db_access; >>>> enum mhi_ee_type ee; >>>> -- >>>> 2.7.4 >>>>
On Wed, Aug 09, 2023 at 11:42:39AM +0800, Qiang Yu wrote: > > On 8/8/2023 6:59 PM, Manivannan Sadhasivam wrote: > > On Tue, Aug 08, 2023 at 04:53:32PM +0800, Qiang Yu wrote: > > > On 8/8/2023 3:51 PM, Manivannan Sadhasivam wrote: > > > > On Tue, Aug 08, 2023 at 10:03:35AM +0800, Qiang Yu wrote: > > > > > Add generic info for SDX75 based modems. SDX75 takes longer than expected > > > > > (default, 8 seconds) to set ready after reboot. Hence add optional ready > > > > > timeout parameter to wait enough for device ready as part of power up > > > > > sequence. > > > > > > > > > > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> > > > > > --- > > > > > drivers/bus/mhi/host/init.c | 1 + > > > > > drivers/bus/mhi/host/main.c | 7 ++++++- > > > > > drivers/bus/mhi/host/pci_generic.c | 22 ++++++++++++++++++++++ > > > > > drivers/bus/mhi/host/pm.c | 6 +++++- > > > > > include/linux/mhi.h | 4 ++++ > > > > > 5 files changed, 38 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c > > > > > index f78aefd..65ceac1 100644 > > > > > --- a/drivers/bus/mhi/host/init.c > > > > > +++ b/drivers/bus/mhi/host/init.c > > > > > @@ -881,6 +881,7 @@ static int parse_config(struct mhi_controller *mhi_cntrl, > > > > > if (!mhi_cntrl->timeout_ms) > > > > > mhi_cntrl->timeout_ms = MHI_TIMEOUT_MS; > > > > > + mhi_cntrl->ready_timeout_ms = config->ready_timeout_ms; > > > > > mhi_cntrl->bounce_buf = config->use_bounce_buf; > > > > > mhi_cntrl->buffer_len = config->buf_len; > > > > > if (!mhi_cntrl->buffer_len) > > > > > diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c > > > > > index 74a7543..8590926 100644 > > > > > --- a/drivers/bus/mhi/host/main.c > > > > > +++ b/drivers/bus/mhi/host/main.c > > > > > @@ -43,8 +43,13 @@ int __must_check mhi_poll_reg_field(struct mhi_controller *mhi_cntrl, > > > > > u32 mask, u32 val, u32 delayus) > > > > > { > > > > > int ret; > > > > > - u32 out, retry = (mhi_cntrl->timeout_ms * 1000) / delayus; > > > > > + u32 out, retry; > > > > > + u32 timeout_ms = mhi_cntrl->timeout_ms; > > > > > + if (mhi_cntrl->ready_timeout_ms && mask == MHISTATUS_READY_MASK) > > > > > + timeout_ms = mhi_cntrl->ready_timeout_ms; > > > > Instead of handling the timeout inside mhi_poll_reg_field(), you should pass the > > > > appropriate timeout value to this function. > > > OK, will do. > > > > > + > > > > > + retry = (timeout_ms * 1000) / delayus; > > > > > while (retry--) { > > > > > ret = mhi_read_reg_field(mhi_cntrl, base, offset, mask, &out); > > > > > if (ret) > > > > > diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c > > > > > index fcd80bc..9c601f0 100644 > > > > > --- a/drivers/bus/mhi/host/pci_generic.c > > > > > +++ b/drivers/bus/mhi/host/pci_generic.c > > > > > @@ -269,6 +269,16 @@ static struct mhi_event_config modem_qcom_v1_mhi_events[] = { > > > > > MHI_EVENT_CONFIG_HW_DATA(5, 2048, 101) > > > > > }; > > > > > +static const struct mhi_controller_config modem_qcom_v2_mhiv_config = { > > > > > + .max_channels = 128, > > > > > + .timeout_ms = 8000, > > > > > + .ready_timeout_ms = 50000, > > > > > + .num_channels = ARRAY_SIZE(modem_qcom_v1_mhi_channels), > > > > > + .ch_cfg = modem_qcom_v1_mhi_channels, > > > > > + .num_events = ARRAY_SIZE(modem_qcom_v1_mhi_events), > > > > > + .event_cfg = modem_qcom_v1_mhi_events, > > > > > +}; > > > > > + > > > > > static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { > > > > > .max_channels = 128, > > > > > .timeout_ms = 8000, > > > > > @@ -278,6 +288,16 @@ static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { > > > > > .event_cfg = modem_qcom_v1_mhi_events, > > > > > }; > > > > > +static const struct mhi_pci_dev_info mhi_qcom_sdx75_info = { > > > > > + .name = "qcom-sdx75m", > > > > > + .fw = "qcom/sdx75m/xbl.elf", > > > > > + .edl = "qcom/sdx75m/edl.mbn", > > > > > + .config = &modem_qcom_v2_mhiv_config, > > > > > + .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > > > > > + .dma_data_width = 32, > > > > > + .sideband_wake = false, > > > > > +}; > > > > > + > > > > > static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = { > > > > > .name = "qcom-sdx65m", > > > > > .fw = "qcom/sdx65m/xbl.elf", > > > > > @@ -597,6 +617,8 @@ static const struct pci_device_id mhi_pci_id_table[] = { > > > > > .driver_data = (kernel_ulong_t) &mhi_telit_fn990_info }, > > > > > { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0308), > > > > > .driver_data = (kernel_ulong_t) &mhi_qcom_sdx65_info }, > > > > > + { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0309), > > > > > + .driver_data = (kernel_ulong_t) &mhi_qcom_sdx75_info }, > > > > > { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1001), /* EM120R-GL (sdx24) */ > > > > > .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info }, > > > > > { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1002), /* EM160R-GL (sdx24) */ > > > > > diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c > > > > > index 8a4362d..6f049e0 100644 > > > > > --- a/drivers/bus/mhi/host/pm.c > > > > > +++ b/drivers/bus/mhi/host/pm.c > > > > > @@ -1202,14 +1202,18 @@ EXPORT_SYMBOL_GPL(mhi_power_down); > > > > > int mhi_sync_power_up(struct mhi_controller *mhi_cntrl) > > > > > { > > > > > int ret = mhi_async_power_up(mhi_cntrl); > > > > > + u32 timeout_ms; > > > > > if (ret) > > > > > return ret; > > > > > + /* Some devices need more time to set ready during power up */ > > > > > + timeout_ms = mhi_cntrl->ready_timeout_ms ? > > > > > + mhi_cntrl->ready_timeout_ms : mhi_cntrl->timeout_ms; > > > > Since you are using this extended timeout value in a couple of places (not just > > > > for checking READY_STATE), it is better to use the existing "timeout_ms" > > > > parameter. > > > > > > > > - Mani > > > We use ready_timeout_ms here is because READY_STATE is polled in a > > > workqueue, in parallel with waiting valid EE. > > > > > > That means we start to wait valid EE and poll ready like at same time > > > instead of starting to wait EE after ready state. > > > > > > Thus the total time it takes to wait valid EE is about the time for polling > > > ready. > > > > > Yes, but why can't you still increase "timeout_ms" for SDX75 and use the same? > > > > Btw, please do not send another version while the discussion is going on for the > > current one. > > > > - Mani > > SDX75 only needs 50 seconds when setting ready for the first time after > power on. Other state transitions > > is expected to wait only 8 seconds. If we use 50s for every state > transition, it's OK but not friendly in some cases. > > > For example, host is resuming from suspend, but device has already crashed > and tansferred to Sahara mode when > > in suspended state. Thus host must wait M0 event timeout, and then reinit > mhi and tranfer to SBL state in recovery > > process. If we set mhi_cntrl->timeout_ms=50s, we have to wait 50s to collect > crash dump after seeing resume fail log. > Hmm. Can't you fix the firmware? Taking 50s to bootup doesn't look good from user perspective. - Mani > > > > > > > wait_event_timeout(mhi_cntrl->state_event, > > > > > MHI_IN_MISSION_MODE(mhi_cntrl->ee) || > > > > > MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state), > > > > > - msecs_to_jiffies(mhi_cntrl->timeout_ms)); > > > > > + msecs_to_jiffies(timeout_ms)); > > > > > ret = (MHI_IN_MISSION_MODE(mhi_cntrl->ee)) ? 0 : -ETIMEDOUT; > > > > > if (ret) > > > > > diff --git a/include/linux/mhi.h b/include/linux/mhi.h > > > > > index f6de4b6..a43e5f8 100644 > > > > > --- a/include/linux/mhi.h > > > > > +++ b/include/linux/mhi.h > > > > > @@ -266,6 +266,7 @@ struct mhi_event_config { > > > > > * struct mhi_controller_config - Root MHI controller configuration > > > > > * @max_channels: Maximum number of channels supported > > > > > * @timeout_ms: Timeout value for operations. 0 means use default > > > > > + * @ready_timeout_ms: Timeout value for waiting device to be ready (optional) > > > > > * @buf_len: Size of automatically allocated buffers. 0 means use default > > > > > * @num_channels: Number of channels defined in @ch_cfg > > > > > * @ch_cfg: Array of defined channels > > > > > @@ -277,6 +278,7 @@ struct mhi_event_config { > > > > > struct mhi_controller_config { > > > > > u32 max_channels; > > > > > u32 timeout_ms; > > > > > + u32 ready_timeout_ms; > > > > > u32 buf_len; > > > > > u32 num_channels; > > > > > const struct mhi_channel_config *ch_cfg; > > > > > @@ -326,6 +328,7 @@ struct mhi_controller_config { > > > > > * @pm_mutex: Mutex for suspend/resume operation > > > > > * @pm_lock: Lock for protecting MHI power management state > > > > > * @timeout_ms: Timeout in ms for state transitions > > > > > + * @ready_timeout_ms: Timeout in ms for waiting device to be ready (optional) > > > > > * @pm_state: MHI power management state > > > > > * @db_access: DB access states > > > > > * @ee: MHI device execution environment > > > > > @@ -413,6 +416,7 @@ struct mhi_controller { > > > > > struct mutex pm_mutex; > > > > > rwlock_t pm_lock; > > > > > u32 timeout_ms; > > > > > + u32 ready_timeout_ms; > > > > > u32 pm_state; > > > > > u32 db_access; > > > > > enum mhi_ee_type ee; > > > > > -- > > > > > 2.7.4 > > > > > >
On 10/17/2023 3:50 PM, Manivannan Sadhasivam wrote: > On Wed, Aug 09, 2023 at 11:42:39AM +0800, Qiang Yu wrote: >> On 8/8/2023 6:59 PM, Manivannan Sadhasivam wrote: >>> On Tue, Aug 08, 2023 at 04:53:32PM +0800, Qiang Yu wrote: >>>> On 8/8/2023 3:51 PM, Manivannan Sadhasivam wrote: >>>>> On Tue, Aug 08, 2023 at 10:03:35AM +0800, Qiang Yu wrote: >>>>>> Add generic info for SDX75 based modems. SDX75 takes longer than expected >>>>>> (default, 8 seconds) to set ready after reboot. Hence add optional ready >>>>>> timeout parameter to wait enough for device ready as part of power up >>>>>> sequence. >>>>>> >>>>>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> >>>>>> --- >>>>>> drivers/bus/mhi/host/init.c | 1 + >>>>>> drivers/bus/mhi/host/main.c | 7 ++++++- >>>>>> drivers/bus/mhi/host/pci_generic.c | 22 ++++++++++++++++++++++ >>>>>> drivers/bus/mhi/host/pm.c | 6 +++++- >>>>>> include/linux/mhi.h | 4 ++++ >>>>>> 5 files changed, 38 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c >>>>>> index f78aefd..65ceac1 100644 >>>>>> --- a/drivers/bus/mhi/host/init.c >>>>>> +++ b/drivers/bus/mhi/host/init.c >>>>>> @@ -881,6 +881,7 @@ static int parse_config(struct mhi_controller *mhi_cntrl, >>>>>> if (!mhi_cntrl->timeout_ms) >>>>>> mhi_cntrl->timeout_ms = MHI_TIMEOUT_MS; >>>>>> + mhi_cntrl->ready_timeout_ms = config->ready_timeout_ms; >>>>>> mhi_cntrl->bounce_buf = config->use_bounce_buf; >>>>>> mhi_cntrl->buffer_len = config->buf_len; >>>>>> if (!mhi_cntrl->buffer_len) >>>>>> diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c >>>>>> index 74a7543..8590926 100644 >>>>>> --- a/drivers/bus/mhi/host/main.c >>>>>> +++ b/drivers/bus/mhi/host/main.c >>>>>> @@ -43,8 +43,13 @@ int __must_check mhi_poll_reg_field(struct mhi_controller *mhi_cntrl, >>>>>> u32 mask, u32 val, u32 delayus) >>>>>> { >>>>>> int ret; >>>>>> - u32 out, retry = (mhi_cntrl->timeout_ms * 1000) / delayus; >>>>>> + u32 out, retry; >>>>>> + u32 timeout_ms = mhi_cntrl->timeout_ms; >>>>>> + if (mhi_cntrl->ready_timeout_ms && mask == MHISTATUS_READY_MASK) >>>>>> + timeout_ms = mhi_cntrl->ready_timeout_ms; >>>>> Instead of handling the timeout inside mhi_poll_reg_field(), you should pass the >>>>> appropriate timeout value to this function. >>>> OK, will do. >>>>>> + >>>>>> + retry = (timeout_ms * 1000) / delayus; >>>>>> while (retry--) { >>>>>> ret = mhi_read_reg_field(mhi_cntrl, base, offset, mask, &out); >>>>>> if (ret) >>>>>> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c >>>>>> index fcd80bc..9c601f0 100644 >>>>>> --- a/drivers/bus/mhi/host/pci_generic.c >>>>>> +++ b/drivers/bus/mhi/host/pci_generic.c >>>>>> @@ -269,6 +269,16 @@ static struct mhi_event_config modem_qcom_v1_mhi_events[] = { >>>>>> MHI_EVENT_CONFIG_HW_DATA(5, 2048, 101) >>>>>> }; >>>>>> +static const struct mhi_controller_config modem_qcom_v2_mhiv_config = { >>>>>> + .max_channels = 128, >>>>>> + .timeout_ms = 8000, >>>>>> + .ready_timeout_ms = 50000, >>>>>> + .num_channels = ARRAY_SIZE(modem_qcom_v1_mhi_channels), >>>>>> + .ch_cfg = modem_qcom_v1_mhi_channels, >>>>>> + .num_events = ARRAY_SIZE(modem_qcom_v1_mhi_events), >>>>>> + .event_cfg = modem_qcom_v1_mhi_events, >>>>>> +}; >>>>>> + >>>>>> static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { >>>>>> .max_channels = 128, >>>>>> .timeout_ms = 8000, >>>>>> @@ -278,6 +288,16 @@ static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { >>>>>> .event_cfg = modem_qcom_v1_mhi_events, >>>>>> }; >>>>>> +static const struct mhi_pci_dev_info mhi_qcom_sdx75_info = { >>>>>> + .name = "qcom-sdx75m", >>>>>> + .fw = "qcom/sdx75m/xbl.elf", >>>>>> + .edl = "qcom/sdx75m/edl.mbn", >>>>>> + .config = &modem_qcom_v2_mhiv_config, >>>>>> + .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >>>>>> + .dma_data_width = 32, >>>>>> + .sideband_wake = false, >>>>>> +}; >>>>>> + >>>>>> static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = { >>>>>> .name = "qcom-sdx65m", >>>>>> .fw = "qcom/sdx65m/xbl.elf", >>>>>> @@ -597,6 +617,8 @@ static const struct pci_device_id mhi_pci_id_table[] = { >>>>>> .driver_data = (kernel_ulong_t) &mhi_telit_fn990_info }, >>>>>> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0308), >>>>>> .driver_data = (kernel_ulong_t) &mhi_qcom_sdx65_info }, >>>>>> + { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0309), >>>>>> + .driver_data = (kernel_ulong_t) &mhi_qcom_sdx75_info }, >>>>>> { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1001), /* EM120R-GL (sdx24) */ >>>>>> .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info }, >>>>>> { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1002), /* EM160R-GL (sdx24) */ >>>>>> diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c >>>>>> index 8a4362d..6f049e0 100644 >>>>>> --- a/drivers/bus/mhi/host/pm.c >>>>>> +++ b/drivers/bus/mhi/host/pm.c >>>>>> @@ -1202,14 +1202,18 @@ EXPORT_SYMBOL_GPL(mhi_power_down); >>>>>> int mhi_sync_power_up(struct mhi_controller *mhi_cntrl) >>>>>> { >>>>>> int ret = mhi_async_power_up(mhi_cntrl); >>>>>> + u32 timeout_ms; >>>>>> if (ret) >>>>>> return ret; >>>>>> + /* Some devices need more time to set ready during power up */ >>>>>> + timeout_ms = mhi_cntrl->ready_timeout_ms ? >>>>>> + mhi_cntrl->ready_timeout_ms : mhi_cntrl->timeout_ms; >>>>> Since you are using this extended timeout value in a couple of places (not just >>>>> for checking READY_STATE), it is better to use the existing "timeout_ms" >>>>> parameter. >>>>> >>>>> - Mani >>>> We use ready_timeout_ms here is because READY_STATE is polled in a >>>> workqueue, in parallel with waiting valid EE. >>>> >>>> That means we start to wait valid EE and poll ready like at same time >>>> instead of starting to wait EE after ready state. >>>> >>>> Thus the total time it takes to wait valid EE is about the time for polling >>>> ready. >>>> >>> Yes, but why can't you still increase "timeout_ms" for SDX75 and use the same? >>> >>> Btw, please do not send another version while the discussion is going on for the >>> current one. >>> >>> - Mani >> SDX75 only needs 50 seconds when setting ready for the first time after >> power on. Other state transitions >> >> is expected to wait only 8 seconds. If we use 50s for every state >> transition, it's OK but not friendly in some cases. >> >> >> For example, host is resuming from suspend, but device has already crashed >> and tansferred to Sahara mode when >> >> in suspended state. Thus host must wait M0 event timeout, and then reinit >> mhi and tranfer to SBL state in recovery >> >> process. If we set mhi_cntrl->timeout_ms=50s, we have to wait 50s to collect >> crash dump after seeing resume fail log. >> > Hmm. Can't you fix the firmware? Taking 50s to bootup doesn't look good from > user perspective. > > - Mani It is a firmware limitation and we can't fix it now. >>>>>> wait_event_timeout(mhi_cntrl->state_event, >>>>>> MHI_IN_MISSION_MODE(mhi_cntrl->ee) || >>>>>> MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state), >>>>>> - msecs_to_jiffies(mhi_cntrl->timeout_ms)); >>>>>> + msecs_to_jiffies(timeout_ms)); >>>>>> ret = (MHI_IN_MISSION_MODE(mhi_cntrl->ee)) ? 0 : -ETIMEDOUT; >>>>>> if (ret) >>>>>> diff --git a/include/linux/mhi.h b/include/linux/mhi.h >>>>>> index f6de4b6..a43e5f8 100644 >>>>>> --- a/include/linux/mhi.h >>>>>> +++ b/include/linux/mhi.h >>>>>> @@ -266,6 +266,7 @@ struct mhi_event_config { >>>>>> * struct mhi_controller_config - Root MHI controller configuration >>>>>> * @max_channels: Maximum number of channels supported >>>>>> * @timeout_ms: Timeout value for operations. 0 means use default >>>>>> + * @ready_timeout_ms: Timeout value for waiting device to be ready (optional) >>>>>> * @buf_len: Size of automatically allocated buffers. 0 means use default >>>>>> * @num_channels: Number of channels defined in @ch_cfg >>>>>> * @ch_cfg: Array of defined channels >>>>>> @@ -277,6 +278,7 @@ struct mhi_event_config { >>>>>> struct mhi_controller_config { >>>>>> u32 max_channels; >>>>>> u32 timeout_ms; >>>>>> + u32 ready_timeout_ms; >>>>>> u32 buf_len; >>>>>> u32 num_channels; >>>>>> const struct mhi_channel_config *ch_cfg; >>>>>> @@ -326,6 +328,7 @@ struct mhi_controller_config { >>>>>> * @pm_mutex: Mutex for suspend/resume operation >>>>>> * @pm_lock: Lock for protecting MHI power management state >>>>>> * @timeout_ms: Timeout in ms for state transitions >>>>>> + * @ready_timeout_ms: Timeout in ms for waiting device to be ready (optional) >>>>>> * @pm_state: MHI power management state >>>>>> * @db_access: DB access states >>>>>> * @ee: MHI device execution environment >>>>>> @@ -413,6 +416,7 @@ struct mhi_controller { >>>>>> struct mutex pm_mutex; >>>>>> rwlock_t pm_lock; >>>>>> u32 timeout_ms; >>>>>> + u32 ready_timeout_ms; >>>>>> u32 pm_state; >>>>>> u32 db_access; >>>>>> enum mhi_ee_type ee; >>>>>> -- >>>>>> 2.7.4 >>>>>>
On Wed, Oct 18, 2023 at 09:52:55AM +0800, Qiang Yu wrote: > > On 10/17/2023 3:50 PM, Manivannan Sadhasivam wrote: > > On Wed, Aug 09, 2023 at 11:42:39AM +0800, Qiang Yu wrote: > > > On 8/8/2023 6:59 PM, Manivannan Sadhasivam wrote: > > > > On Tue, Aug 08, 2023 at 04:53:32PM +0800, Qiang Yu wrote: > > > > > On 8/8/2023 3:51 PM, Manivannan Sadhasivam wrote: > > > > > > On Tue, Aug 08, 2023 at 10:03:35AM +0800, Qiang Yu wrote: > > > > > > > Add generic info for SDX75 based modems. SDX75 takes longer than expected > > > > > > > (default, 8 seconds) to set ready after reboot. Hence add optional ready > > > > > > > timeout parameter to wait enough for device ready as part of power up > > > > > > > sequence. > > > > > > > > > > > > > > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> > > > > > > > --- > > > > > > > drivers/bus/mhi/host/init.c | 1 + > > > > > > > drivers/bus/mhi/host/main.c | 7 ++++++- > > > > > > > drivers/bus/mhi/host/pci_generic.c | 22 ++++++++++++++++++++++ > > > > > > > drivers/bus/mhi/host/pm.c | 6 +++++- > > > > > > > include/linux/mhi.h | 4 ++++ > > > > > > > 5 files changed, 38 insertions(+), 2 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c > > > > > > > index f78aefd..65ceac1 100644 > > > > > > > --- a/drivers/bus/mhi/host/init.c > > > > > > > +++ b/drivers/bus/mhi/host/init.c > > > > > > > @@ -881,6 +881,7 @@ static int parse_config(struct mhi_controller *mhi_cntrl, > > > > > > > if (!mhi_cntrl->timeout_ms) > > > > > > > mhi_cntrl->timeout_ms = MHI_TIMEOUT_MS; > > > > > > > + mhi_cntrl->ready_timeout_ms = config->ready_timeout_ms; > > > > > > > mhi_cntrl->bounce_buf = config->use_bounce_buf; > > > > > > > mhi_cntrl->buffer_len = config->buf_len; > > > > > > > if (!mhi_cntrl->buffer_len) > > > > > > > diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c > > > > > > > index 74a7543..8590926 100644 > > > > > > > --- a/drivers/bus/mhi/host/main.c > > > > > > > +++ b/drivers/bus/mhi/host/main.c > > > > > > > @@ -43,8 +43,13 @@ int __must_check mhi_poll_reg_field(struct mhi_controller *mhi_cntrl, > > > > > > > u32 mask, u32 val, u32 delayus) > > > > > > > { > > > > > > > int ret; > > > > > > > - u32 out, retry = (mhi_cntrl->timeout_ms * 1000) / delayus; > > > > > > > + u32 out, retry; > > > > > > > + u32 timeout_ms = mhi_cntrl->timeout_ms; > > > > > > > + if (mhi_cntrl->ready_timeout_ms && mask == MHISTATUS_READY_MASK) > > > > > > > + timeout_ms = mhi_cntrl->ready_timeout_ms; > > > > > > Instead of handling the timeout inside mhi_poll_reg_field(), you should pass the > > > > > > appropriate timeout value to this function. > > > > > OK, will do. > > > > > > > + > > > > > > > + retry = (timeout_ms * 1000) / delayus; > > > > > > > while (retry--) { > > > > > > > ret = mhi_read_reg_field(mhi_cntrl, base, offset, mask, &out); > > > > > > > if (ret) > > > > > > > diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c > > > > > > > index fcd80bc..9c601f0 100644 > > > > > > > --- a/drivers/bus/mhi/host/pci_generic.c > > > > > > > +++ b/drivers/bus/mhi/host/pci_generic.c > > > > > > > @@ -269,6 +269,16 @@ static struct mhi_event_config modem_qcom_v1_mhi_events[] = { > > > > > > > MHI_EVENT_CONFIG_HW_DATA(5, 2048, 101) > > > > > > > }; > > > > > > > +static const struct mhi_controller_config modem_qcom_v2_mhiv_config = { > > > > > > > + .max_channels = 128, > > > > > > > + .timeout_ms = 8000, > > > > > > > + .ready_timeout_ms = 50000, > > > > > > > + .num_channels = ARRAY_SIZE(modem_qcom_v1_mhi_channels), > > > > > > > + .ch_cfg = modem_qcom_v1_mhi_channels, > > > > > > > + .num_events = ARRAY_SIZE(modem_qcom_v1_mhi_events), > > > > > > > + .event_cfg = modem_qcom_v1_mhi_events, > > > > > > > +}; > > > > > > > + > > > > > > > static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { > > > > > > > .max_channels = 128, > > > > > > > .timeout_ms = 8000, > > > > > > > @@ -278,6 +288,16 @@ static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { > > > > > > > .event_cfg = modem_qcom_v1_mhi_events, > > > > > > > }; > > > > > > > +static const struct mhi_pci_dev_info mhi_qcom_sdx75_info = { > > > > > > > + .name = "qcom-sdx75m", > > > > > > > + .fw = "qcom/sdx75m/xbl.elf", > > > > > > > + .edl = "qcom/sdx75m/edl.mbn", > > > > > > > + .config = &modem_qcom_v2_mhiv_config, > > > > > > > + .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > > > > > > > + .dma_data_width = 32, > > > > > > > + .sideband_wake = false, > > > > > > > +}; > > > > > > > + > > > > > > > static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = { > > > > > > > .name = "qcom-sdx65m", > > > > > > > .fw = "qcom/sdx65m/xbl.elf", > > > > > > > @@ -597,6 +617,8 @@ static const struct pci_device_id mhi_pci_id_table[] = { > > > > > > > .driver_data = (kernel_ulong_t) &mhi_telit_fn990_info }, > > > > > > > { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0308), > > > > > > > .driver_data = (kernel_ulong_t) &mhi_qcom_sdx65_info }, > > > > > > > + { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0309), > > > > > > > + .driver_data = (kernel_ulong_t) &mhi_qcom_sdx75_info }, > > > > > > > { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1001), /* EM120R-GL (sdx24) */ > > > > > > > .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info }, > > > > > > > { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1002), /* EM160R-GL (sdx24) */ > > > > > > > diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c > > > > > > > index 8a4362d..6f049e0 100644 > > > > > > > --- a/drivers/bus/mhi/host/pm.c > > > > > > > +++ b/drivers/bus/mhi/host/pm.c > > > > > > > @@ -1202,14 +1202,18 @@ EXPORT_SYMBOL_GPL(mhi_power_down); > > > > > > > int mhi_sync_power_up(struct mhi_controller *mhi_cntrl) > > > > > > > { > > > > > > > int ret = mhi_async_power_up(mhi_cntrl); > > > > > > > + u32 timeout_ms; > > > > > > > if (ret) > > > > > > > return ret; > > > > > > > + /* Some devices need more time to set ready during power up */ > > > > > > > + timeout_ms = mhi_cntrl->ready_timeout_ms ? > > > > > > > + mhi_cntrl->ready_timeout_ms : mhi_cntrl->timeout_ms; > > > > > > Since you are using this extended timeout value in a couple of places (not just > > > > > > for checking READY_STATE), it is better to use the existing "timeout_ms" > > > > > > parameter. > > > > > > > > > > > > - Mani > > > > > We use ready_timeout_ms here is because READY_STATE is polled in a > > > > > workqueue, in parallel with waiting valid EE. > > > > > > > > > > That means we start to wait valid EE and poll ready like at same time > > > > > instead of starting to wait EE after ready state. > > > > > > > > > > Thus the total time it takes to wait valid EE is about the time for polling > > > > > ready. > > > > > > > > > Yes, but why can't you still increase "timeout_ms" for SDX75 and use the same? > > > > > > > > Btw, please do not send another version while the discussion is going on for the > > > > current one. > > > > > > > > - Mani > > > SDX75 only needs 50 seconds when setting ready for the first time after > > > power on. Other state transitions > > > > > > is expected to wait only 8 seconds. If we use 50s for every state > > > transition, it's OK but not friendly in some cases. > > > > > > > > > For example, host is resuming from suspend, but device has already crashed > > > and tansferred to Sahara mode when > > > > > > in suspended state. Thus host must wait M0 event timeout, and then reinit > > > mhi and tranfer to SBL state in recovery > > > > > > process. If we set mhi_cntrl->timeout_ms=50s, we have to wait 50s to collect > > > crash dump after seeing resume fail log. > > > > > Hmm. Can't you fix the firmware? Taking 50s to bootup doesn't look good from > > user perspective. > > > > - Mani > It is a firmware limitation and we can't fix it now. Okay. Then I'm fine with this workaround. Please post the next version incorporating other comments. - Mani > > > > > > > wait_event_timeout(mhi_cntrl->state_event, > > > > > > > MHI_IN_MISSION_MODE(mhi_cntrl->ee) || > > > > > > > MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state), > > > > > > > - msecs_to_jiffies(mhi_cntrl->timeout_ms)); > > > > > > > + msecs_to_jiffies(timeout_ms)); > > > > > > > ret = (MHI_IN_MISSION_MODE(mhi_cntrl->ee)) ? 0 : -ETIMEDOUT; > > > > > > > if (ret) > > > > > > > diff --git a/include/linux/mhi.h b/include/linux/mhi.h > > > > > > > index f6de4b6..a43e5f8 100644 > > > > > > > --- a/include/linux/mhi.h > > > > > > > +++ b/include/linux/mhi.h > > > > > > > @@ -266,6 +266,7 @@ struct mhi_event_config { > > > > > > > * struct mhi_controller_config - Root MHI controller configuration > > > > > > > * @max_channels: Maximum number of channels supported > > > > > > > * @timeout_ms: Timeout value for operations. 0 means use default > > > > > > > + * @ready_timeout_ms: Timeout value for waiting device to be ready (optional) > > > > > > > * @buf_len: Size of automatically allocated buffers. 0 means use default > > > > > > > * @num_channels: Number of channels defined in @ch_cfg > > > > > > > * @ch_cfg: Array of defined channels > > > > > > > @@ -277,6 +278,7 @@ struct mhi_event_config { > > > > > > > struct mhi_controller_config { > > > > > > > u32 max_channels; > > > > > > > u32 timeout_ms; > > > > > > > + u32 ready_timeout_ms; > > > > > > > u32 buf_len; > > > > > > > u32 num_channels; > > > > > > > const struct mhi_channel_config *ch_cfg; > > > > > > > @@ -326,6 +328,7 @@ struct mhi_controller_config { > > > > > > > * @pm_mutex: Mutex for suspend/resume operation > > > > > > > * @pm_lock: Lock for protecting MHI power management state > > > > > > > * @timeout_ms: Timeout in ms for state transitions > > > > > > > + * @ready_timeout_ms: Timeout in ms for waiting device to be ready (optional) > > > > > > > * @pm_state: MHI power management state > > > > > > > * @db_access: DB access states > > > > > > > * @ee: MHI device execution environment > > > > > > > @@ -413,6 +416,7 @@ struct mhi_controller { > > > > > > > struct mutex pm_mutex; > > > > > > > rwlock_t pm_lock; > > > > > > > u32 timeout_ms; > > > > > > > + u32 ready_timeout_ms; > > > > > > > u32 pm_state; > > > > > > > u32 db_access; > > > > > > > enum mhi_ee_type ee; > > > > > > > -- > > > > > > > 2.7.4 > > > > > > >
On 10/18/2023 9:08 PM, Manivannan Sadhasivam wrote: > On Wed, Oct 18, 2023 at 09:52:55AM +0800, Qiang Yu wrote: >> On 10/17/2023 3:50 PM, Manivannan Sadhasivam wrote: >>> On Wed, Aug 09, 2023 at 11:42:39AM +0800, Qiang Yu wrote: >>>> On 8/8/2023 6:59 PM, Manivannan Sadhasivam wrote: >>>>> On Tue, Aug 08, 2023 at 04:53:32PM +0800, Qiang Yu wrote: >>>>>> On 8/8/2023 3:51 PM, Manivannan Sadhasivam wrote: >>>>>>> On Tue, Aug 08, 2023 at 10:03:35AM +0800, Qiang Yu wrote: >>>>>>>> Add generic info for SDX75 based modems. SDX75 takes longer than expected >>>>>>>> (default, 8 seconds) to set ready after reboot. Hence add optional ready >>>>>>>> timeout parameter to wait enough for device ready as part of power up >>>>>>>> sequence. >>>>>>>> >>>>>>>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> >>>>>>>> --- >>>>>>>> drivers/bus/mhi/host/init.c | 1 + >>>>>>>> drivers/bus/mhi/host/main.c | 7 ++++++- >>>>>>>> drivers/bus/mhi/host/pci_generic.c | 22 ++++++++++++++++++++++ >>>>>>>> drivers/bus/mhi/host/pm.c | 6 +++++- >>>>>>>> include/linux/mhi.h | 4 ++++ >>>>>>>> 5 files changed, 38 insertions(+), 2 deletions(-) >>>>>>>> >>>>>>>> diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c >>>>>>>> index f78aefd..65ceac1 100644 >>>>>>>> --- a/drivers/bus/mhi/host/init.c >>>>>>>> +++ b/drivers/bus/mhi/host/init.c >>>>>>>> @@ -881,6 +881,7 @@ static int parse_config(struct mhi_controller *mhi_cntrl, >>>>>>>> if (!mhi_cntrl->timeout_ms) >>>>>>>> mhi_cntrl->timeout_ms = MHI_TIMEOUT_MS; >>>>>>>> + mhi_cntrl->ready_timeout_ms = config->ready_timeout_ms; >>>>>>>> mhi_cntrl->bounce_buf = config->use_bounce_buf; >>>>>>>> mhi_cntrl->buffer_len = config->buf_len; >>>>>>>> if (!mhi_cntrl->buffer_len) >>>>>>>> diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c >>>>>>>> index 74a7543..8590926 100644 >>>>>>>> --- a/drivers/bus/mhi/host/main.c >>>>>>>> +++ b/drivers/bus/mhi/host/main.c >>>>>>>> @@ -43,8 +43,13 @@ int __must_check mhi_poll_reg_field(struct mhi_controller *mhi_cntrl, >>>>>>>> u32 mask, u32 val, u32 delayus) >>>>>>>> { >>>>>>>> int ret; >>>>>>>> - u32 out, retry = (mhi_cntrl->timeout_ms * 1000) / delayus; >>>>>>>> + u32 out, retry; >>>>>>>> + u32 timeout_ms = mhi_cntrl->timeout_ms; >>>>>>>> + if (mhi_cntrl->ready_timeout_ms && mask == MHISTATUS_READY_MASK) >>>>>>>> + timeout_ms = mhi_cntrl->ready_timeout_ms; >>>>>>> Instead of handling the timeout inside mhi_poll_reg_field(), you should pass the >>>>>>> appropriate timeout value to this function. >>>>>> OK, will do. >>>>>>>> + >>>>>>>> + retry = (timeout_ms * 1000) / delayus; >>>>>>>> while (retry--) { >>>>>>>> ret = mhi_read_reg_field(mhi_cntrl, base, offset, mask, &out); >>>>>>>> if (ret) >>>>>>>> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c >>>>>>>> index fcd80bc..9c601f0 100644 >>>>>>>> --- a/drivers/bus/mhi/host/pci_generic.c >>>>>>>> +++ b/drivers/bus/mhi/host/pci_generic.c >>>>>>>> @@ -269,6 +269,16 @@ static struct mhi_event_config modem_qcom_v1_mhi_events[] = { >>>>>>>> MHI_EVENT_CONFIG_HW_DATA(5, 2048, 101) >>>>>>>> }; >>>>>>>> +static const struct mhi_controller_config modem_qcom_v2_mhiv_config = { >>>>>>>> + .max_channels = 128, >>>>>>>> + .timeout_ms = 8000, >>>>>>>> + .ready_timeout_ms = 50000, >>>>>>>> + .num_channels = ARRAY_SIZE(modem_qcom_v1_mhi_channels), >>>>>>>> + .ch_cfg = modem_qcom_v1_mhi_channels, >>>>>>>> + .num_events = ARRAY_SIZE(modem_qcom_v1_mhi_events), >>>>>>>> + .event_cfg = modem_qcom_v1_mhi_events, >>>>>>>> +}; >>>>>>>> + >>>>>>>> static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { >>>>>>>> .max_channels = 128, >>>>>>>> .timeout_ms = 8000, >>>>>>>> @@ -278,6 +288,16 @@ static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { >>>>>>>> .event_cfg = modem_qcom_v1_mhi_events, >>>>>>>> }; >>>>>>>> +static const struct mhi_pci_dev_info mhi_qcom_sdx75_info = { >>>>>>>> + .name = "qcom-sdx75m", >>>>>>>> + .fw = "qcom/sdx75m/xbl.elf", >>>>>>>> + .edl = "qcom/sdx75m/edl.mbn", >>>>>>>> + .config = &modem_qcom_v2_mhiv_config, >>>>>>>> + .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >>>>>>>> + .dma_data_width = 32, >>>>>>>> + .sideband_wake = false, >>>>>>>> +}; >>>>>>>> + >>>>>>>> static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = { >>>>>>>> .name = "qcom-sdx65m", >>>>>>>> .fw = "qcom/sdx65m/xbl.elf", >>>>>>>> @@ -597,6 +617,8 @@ static const struct pci_device_id mhi_pci_id_table[] = { >>>>>>>> .driver_data = (kernel_ulong_t) &mhi_telit_fn990_info }, >>>>>>>> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0308), >>>>>>>> .driver_data = (kernel_ulong_t) &mhi_qcom_sdx65_info }, >>>>>>>> + { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0309), >>>>>>>> + .driver_data = (kernel_ulong_t) &mhi_qcom_sdx75_info }, >>>>>>>> { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1001), /* EM120R-GL (sdx24) */ >>>>>>>> .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info }, >>>>>>>> { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1002), /* EM160R-GL (sdx24) */ >>>>>>>> diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c >>>>>>>> index 8a4362d..6f049e0 100644 >>>>>>>> --- a/drivers/bus/mhi/host/pm.c >>>>>>>> +++ b/drivers/bus/mhi/host/pm.c >>>>>>>> @@ -1202,14 +1202,18 @@ EXPORT_SYMBOL_GPL(mhi_power_down); >>>>>>>> int mhi_sync_power_up(struct mhi_controller *mhi_cntrl) >>>>>>>> { >>>>>>>> int ret = mhi_async_power_up(mhi_cntrl); >>>>>>>> + u32 timeout_ms; >>>>>>>> if (ret) >>>>>>>> return ret; >>>>>>>> + /* Some devices need more time to set ready during power up */ >>>>>>>> + timeout_ms = mhi_cntrl->ready_timeout_ms ? >>>>>>>> + mhi_cntrl->ready_timeout_ms : mhi_cntrl->timeout_ms; >>>>>>> Since you are using this extended timeout value in a couple of places (not just >>>>>>> for checking READY_STATE), it is better to use the existing "timeout_ms" >>>>>>> parameter. >>>>>>> >>>>>>> - Mani >>>>>> We use ready_timeout_ms here is because READY_STATE is polled in a >>>>>> workqueue, in parallel with waiting valid EE. >>>>>> >>>>>> That means we start to wait valid EE and poll ready like at same time >>>>>> instead of starting to wait EE after ready state. >>>>>> >>>>>> Thus the total time it takes to wait valid EE is about the time for polling >>>>>> ready. >>>>>> >>>>> Yes, but why can't you still increase "timeout_ms" for SDX75 and use the same? >>>>> >>>>> Btw, please do not send another version while the discussion is going on for the >>>>> current one. >>>>> >>>>> - Mani >>>> SDX75 only needs 50 seconds when setting ready for the first time after >>>> power on. Other state transitions >>>> >>>> is expected to wait only 8 seconds. If we use 50s for every state >>>> transition, it's OK but not friendly in some cases. >>>> >>>> >>>> For example, host is resuming from suspend, but device has already crashed >>>> and tansferred to Sahara mode when >>>> >>>> in suspended state. Thus host must wait M0 event timeout, and then reinit >>>> mhi and tranfer to SBL state in recovery >>>> >>>> process. If we set mhi_cntrl->timeout_ms=50s, we have to wait 50s to collect >>>> crash dump after seeing resume fail log. >>>> >>> Hmm. Can't you fix the firmware? Taking 50s to bootup doesn't look good from >>> user perspective. >>> >>> - Mani >> It is a firmware limitation and we can't fix it now. > Okay. Then I'm fine with this workaround. > > Please post the next version incorporating other comments. > > - Mani Hi Mani, actually I have sent [patch v2] before we complete the discussion on current patch. The [patch v2] has incorporated the comments that need to add changes in the patch. If I send next version patch, I will just change the subject-prefix to v3, without any code or commit message change. Is this acceptable? Or Could you please review the [patch v2]? https://lore.kernel.org/mhi/1691488809-85310-1-git-send-email-quic_qianyu@quicinc.com/T/#u >>>>>>>> wait_event_timeout(mhi_cntrl->state_event, >>>>>>>> MHI_IN_MISSION_MODE(mhi_cntrl->ee) || >>>>>>>> MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state), >>>>>>>> - msecs_to_jiffies(mhi_cntrl->timeout_ms)); >>>>>>>> + msecs_to_jiffies(timeout_ms)); >>>>>>>> ret = (MHI_IN_MISSION_MODE(mhi_cntrl->ee)) ? 0 : -ETIMEDOUT; >>>>>>>> if (ret) >>>>>>>> diff --git a/include/linux/mhi.h b/include/linux/mhi.h >>>>>>>> index f6de4b6..a43e5f8 100644 >>>>>>>> --- a/include/linux/mhi.h >>>>>>>> +++ b/include/linux/mhi.h >>>>>>>> @@ -266,6 +266,7 @@ struct mhi_event_config { >>>>>>>> * struct mhi_controller_config - Root MHI controller configuration >>>>>>>> * @max_channels: Maximum number of channels supported >>>>>>>> * @timeout_ms: Timeout value for operations. 0 means use default >>>>>>>> + * @ready_timeout_ms: Timeout value for waiting device to be ready (optional) >>>>>>>> * @buf_len: Size of automatically allocated buffers. 0 means use default >>>>>>>> * @num_channels: Number of channels defined in @ch_cfg >>>>>>>> * @ch_cfg: Array of defined channels >>>>>>>> @@ -277,6 +278,7 @@ struct mhi_event_config { >>>>>>>> struct mhi_controller_config { >>>>>>>> u32 max_channels; >>>>>>>> u32 timeout_ms; >>>>>>>> + u32 ready_timeout_ms; >>>>>>>> u32 buf_len; >>>>>>>> u32 num_channels; >>>>>>>> const struct mhi_channel_config *ch_cfg; >>>>>>>> @@ -326,6 +328,7 @@ struct mhi_controller_config { >>>>>>>> * @pm_mutex: Mutex for suspend/resume operation >>>>>>>> * @pm_lock: Lock for protecting MHI power management state >>>>>>>> * @timeout_ms: Timeout in ms for state transitions >>>>>>>> + * @ready_timeout_ms: Timeout in ms for waiting device to be ready (optional) >>>>>>>> * @pm_state: MHI power management state >>>>>>>> * @db_access: DB access states >>>>>>>> * @ee: MHI device execution environment >>>>>>>> @@ -413,6 +416,7 @@ struct mhi_controller { >>>>>>>> struct mutex pm_mutex; >>>>>>>> rwlock_t pm_lock; >>>>>>>> u32 timeout_ms; >>>>>>>> + u32 ready_timeout_ms; >>>>>>>> u32 pm_state; >>>>>>>> u32 db_access; >>>>>>>> enum mhi_ee_type ee; >>>>>>>> -- >>>>>>>> 2.7.4 >>>>>>>>
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c index f78aefd..65ceac1 100644 --- a/drivers/bus/mhi/host/init.c +++ b/drivers/bus/mhi/host/init.c @@ -881,6 +881,7 @@ static int parse_config(struct mhi_controller *mhi_cntrl, if (!mhi_cntrl->timeout_ms) mhi_cntrl->timeout_ms = MHI_TIMEOUT_MS; + mhi_cntrl->ready_timeout_ms = config->ready_timeout_ms; mhi_cntrl->bounce_buf = config->use_bounce_buf; mhi_cntrl->buffer_len = config->buf_len; if (!mhi_cntrl->buffer_len) diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c index 74a7543..8590926 100644 --- a/drivers/bus/mhi/host/main.c +++ b/drivers/bus/mhi/host/main.c @@ -43,8 +43,13 @@ int __must_check mhi_poll_reg_field(struct mhi_controller *mhi_cntrl, u32 mask, u32 val, u32 delayus) { int ret; - u32 out, retry = (mhi_cntrl->timeout_ms * 1000) / delayus; + u32 out, retry; + u32 timeout_ms = mhi_cntrl->timeout_ms; + if (mhi_cntrl->ready_timeout_ms && mask == MHISTATUS_READY_MASK) + timeout_ms = mhi_cntrl->ready_timeout_ms; + + retry = (timeout_ms * 1000) / delayus; while (retry--) { ret = mhi_read_reg_field(mhi_cntrl, base, offset, mask, &out); if (ret) diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c index fcd80bc..9c601f0 100644 --- a/drivers/bus/mhi/host/pci_generic.c +++ b/drivers/bus/mhi/host/pci_generic.c @@ -269,6 +269,16 @@ static struct mhi_event_config modem_qcom_v1_mhi_events[] = { MHI_EVENT_CONFIG_HW_DATA(5, 2048, 101) }; +static const struct mhi_controller_config modem_qcom_v2_mhiv_config = { + .max_channels = 128, + .timeout_ms = 8000, + .ready_timeout_ms = 50000, + .num_channels = ARRAY_SIZE(modem_qcom_v1_mhi_channels), + .ch_cfg = modem_qcom_v1_mhi_channels, + .num_events = ARRAY_SIZE(modem_qcom_v1_mhi_events), + .event_cfg = modem_qcom_v1_mhi_events, +}; + static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { .max_channels = 128, .timeout_ms = 8000, @@ -278,6 +288,16 @@ static const struct mhi_controller_config modem_qcom_v1_mhiv_config = { .event_cfg = modem_qcom_v1_mhi_events, }; +static const struct mhi_pci_dev_info mhi_qcom_sdx75_info = { + .name = "qcom-sdx75m", + .fw = "qcom/sdx75m/xbl.elf", + .edl = "qcom/sdx75m/edl.mbn", + .config = &modem_qcom_v2_mhiv_config, + .bar_num = MHI_PCI_DEFAULT_BAR_NUM, + .dma_data_width = 32, + .sideband_wake = false, +}; + static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = { .name = "qcom-sdx65m", .fw = "qcom/sdx65m/xbl.elf", @@ -597,6 +617,8 @@ static const struct pci_device_id mhi_pci_id_table[] = { .driver_data = (kernel_ulong_t) &mhi_telit_fn990_info }, { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0308), .driver_data = (kernel_ulong_t) &mhi_qcom_sdx65_info }, + { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0309), + .driver_data = (kernel_ulong_t) &mhi_qcom_sdx75_info }, { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1001), /* EM120R-GL (sdx24) */ .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info }, { PCI_DEVICE(PCI_VENDOR_ID_QUECTEL, 0x1002), /* EM160R-GL (sdx24) */ diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c index 8a4362d..6f049e0 100644 --- a/drivers/bus/mhi/host/pm.c +++ b/drivers/bus/mhi/host/pm.c @@ -1202,14 +1202,18 @@ EXPORT_SYMBOL_GPL(mhi_power_down); int mhi_sync_power_up(struct mhi_controller *mhi_cntrl) { int ret = mhi_async_power_up(mhi_cntrl); + u32 timeout_ms; if (ret) return ret; + /* Some devices need more time to set ready during power up */ + timeout_ms = mhi_cntrl->ready_timeout_ms ? + mhi_cntrl->ready_timeout_ms : mhi_cntrl->timeout_ms; wait_event_timeout(mhi_cntrl->state_event, MHI_IN_MISSION_MODE(mhi_cntrl->ee) || MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state), - msecs_to_jiffies(mhi_cntrl->timeout_ms)); + msecs_to_jiffies(timeout_ms)); ret = (MHI_IN_MISSION_MODE(mhi_cntrl->ee)) ? 0 : -ETIMEDOUT; if (ret) diff --git a/include/linux/mhi.h b/include/linux/mhi.h index f6de4b6..a43e5f8 100644 --- a/include/linux/mhi.h +++ b/include/linux/mhi.h @@ -266,6 +266,7 @@ struct mhi_event_config { * struct mhi_controller_config - Root MHI controller configuration * @max_channels: Maximum number of channels supported * @timeout_ms: Timeout value for operations. 0 means use default + * @ready_timeout_ms: Timeout value for waiting device to be ready (optional) * @buf_len: Size of automatically allocated buffers. 0 means use default * @num_channels: Number of channels defined in @ch_cfg * @ch_cfg: Array of defined channels @@ -277,6 +278,7 @@ struct mhi_event_config { struct mhi_controller_config { u32 max_channels; u32 timeout_ms; + u32 ready_timeout_ms; u32 buf_len; u32 num_channels; const struct mhi_channel_config *ch_cfg; @@ -326,6 +328,7 @@ struct mhi_controller_config { * @pm_mutex: Mutex for suspend/resume operation * @pm_lock: Lock for protecting MHI power management state * @timeout_ms: Timeout in ms for state transitions + * @ready_timeout_ms: Timeout in ms for waiting device to be ready (optional) * @pm_state: MHI power management state * @db_access: DB access states * @ee: MHI device execution environment @@ -413,6 +416,7 @@ struct mhi_controller { struct mutex pm_mutex; rwlock_t pm_lock; u32 timeout_ms; + u32 ready_timeout_ms; u32 pm_state; u32 db_access; enum mhi_ee_type ee;