Re: [PATCH] bus: mhi: pci_generic: increase timeout value for operations to 24000ms

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To: Aleksander Morgado <aleksander@aleksander.es>,
	Thomas Perrot <thomas.perrot@bootlin.com>,
	linux-arm-msm@vger.kernel.org
Cc: hemantk@codeaurora.org, loic.poulain@linaro.org, stable@vger.kernel.org
Subject: Re: [PATCH] bus: mhi: pci_generic: increase timeout value for operations to 24000ms
Date: Wed, 29 Sep 2021 17:34:42 +0530	[thread overview]
Message-ID: <73A52D61-FCAB-4A2B-BA96-0117F6942842@linaro.org> (raw)
In-Reply-To: <f358044a-78d0-ad63-a777-87b4b9d94745@aleksander.es>

On 29 September 2021 3:07:23 PM IST, Aleksander Morgado <aleksander@aleksander.es> wrote:
>Hey all,
>
>On 5/8/21 16:02, Thomas Perrot wrote:
>> Otherwise, the waiting time was too short to use a Sierra Wireless EM919X
>> connected to an i.MX6 through the PCIe bus.
>> 
>> Signed-off-by: Thomas Perrot <thomas.perrot@bootlin.com>
>> ---
>>   drivers/bus/mhi/pci_generic.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c
>> index 4dd1077354af..e08ed6e5031b 100644
>> --- a/drivers/bus/mhi/pci_generic.c
>> +++ b/drivers/bus/mhi/pci_generic.c
>> @@ -248,7 +248,7 @@ static struct mhi_event_config modem_qcom_v1_mhi_events[] = {
>>   
>>   static const struct mhi_controller_config modem_qcom_v1_mhiv_config = {
>>   	.max_channels = 128,
>> -	.timeout_ms = 8000,
>> +	.timeout_ms = 24000,
>
>
>This modem_qcom_v1_mhiv_config config applies to all generic SDX24, SDX55 and SDX65 modules.
>Other vendor-branded SDX55 based modules in this same file (Foxconn SDX55, MV31), have 20000ms as timeout.
>Other vendor-branded SDX24 based modules in this same file (Quectel EM12xx), have also 20000ms as timeout.
>Maybe it makes sense to have a common timeout for all?
>

Eventhough the baseport coming from Qualcomm for the modem chipsets are same, it is possible that the vendors might have customized the firmware for their own usecase. That could be the cause of the delay for modem booting. 

So I don't think we should use the same timeout of 2400ms for all modems. 

>Thomas, is the 24000ms value taken from experimentation, or is it a safe enough value? Maybe 20000ms as in other modules would have been enough?
>

It was derived from testing I believe. 

Thanks, 
Mani

>And if 20000ms wasn't enough but 24000ms is, how about adding that same value for all modules? These modules definitely need time to boot, not sure if having slightly different timeout values for each would make much sense, unless there are very very different values required.
>
>What do you think?
>
>>   	.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),
>> 
>
>

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.