[PATCH v2] soundwire: qcom: remove redundant wait for completion

[PATCH v2] soundwire: qcom: remove redundant wait for completion

[Srinivasa Rao Mandadapu]

Remove wait_for_completion_timeout from soundwire probe as it seems
unnecessary and device enumeration is anyway not happening here,
hence this api is blocking till it completes max wait time.
Also, as device enumeration event is dependent on wcd938x probe to be
completed, its of no use waiting here.
Waiting here increasing the boot time almost 4 seconds and impacting
other modules like touch screen.

Fixes: 06dd96738d618 ("soundwire: qcom: wait for enumeration to be complete in probe")

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
---
 drivers/soundwire/qcom.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 5481341..9a32a24 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -1309,8 +1309,6 @@ static int qcom_swrm_probe(struct platform_device *pdev)
 	}
 
 	qcom_swrm_init(ctrl);
-	wait_for_completion_timeout(&ctrl->enumeration,
-				    msecs_to_jiffies(TIMEOUT_MS));
 	ret = qcom_swrm_register_dais(ctrl);
 	if (ret)
 		goto err_master_add;
-- 
2.7.4

Re: [PATCH v2] soundwire: qcom: remove redundant wait for completion

[Srinivas Kandagatla]

On 25/02/2022 14:44, Srinivasa Rao Mandadapu wrote:
> Remove wait_for_completion_timeout from soundwire probe as it seems
> unnecessary and device enumeration is anyway not happening here,
> hence this api is blocking till it completes max wait time.
> Also, as device enumeration event is dependent on wcd938x probe to be
> completed, its of no use waiting here.
> Waiting here increasing the boot time almost 4 seconds and impacting
> other modules like touch screen.
> 
> Fixes: 06dd96738d618 ("soundwire: qcom: wait for enumeration to be complete in probe")
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>

LGTM,

Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>


> ---
>   drivers/soundwire/qcom.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 5481341..9a32a24 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -1309,8 +1309,6 @@ static int qcom_swrm_probe(struct platform_device *pdev)
>   	}
>   
>   	qcom_swrm_init(ctrl);
> -	wait_for_completion_timeout(&ctrl->enumeration,
> -				    msecs_to_jiffies(TIMEOUT_MS));
>   	ret = qcom_swrm_register_dais(ctrl);
>   	if (ret)
>   		goto err_master_add;

Re: [PATCH v2] soundwire: qcom: remove redundant wait for completion

[Pierre-Louis Bossart]

On 2/25/22 08:45, Srinivas Kandagatla wrote:
> 
> 
> On 25/02/2022 14:44, Srinivasa Rao Mandadapu wrote:
>> Remove wait_for_completion_timeout from soundwire probe as it seems
>> unnecessary and device enumeration is anyway not happening here,
>> hence this api is blocking till it completes max wait time.
>> Also, as device enumeration event is dependent on wcd938x probe to be
>> completed, its of no use waiting here.
>> Waiting here increasing the boot time almost 4 seconds and impacting
>> other modules like touch screen.
>>
>> Fixes: 06dd96738d618 ("soundwire: qcom: wait for enumeration to be
>> complete in probe")
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> 
> LGTM,
> 
> Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

I don't get the idea, sorry.

If you look at the code, these are the cases where this 'struct
completion' is used

	struct completion enumeration;
	complete(&ctrl->enumeration);
	/* Enable Auto enumeration */
	init_completion(&ctrl->enumeration);
	wait_for_completion_timeout(&ctrl->enumeration,


so if you remove the wait_for_completeion, then you might just as well
remove the whole thing and revert 06dd96738d618

what am I missing?


>> ---
>>   drivers/soundwire/qcom.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
>> index 5481341..9a32a24 100644
>> --- a/drivers/soundwire/qcom.c
>> +++ b/drivers/soundwire/qcom.c
>> @@ -1309,8 +1309,6 @@ static int qcom_swrm_probe(struct
>> platform_device *pdev)
>>       }
>>         qcom_swrm_init(ctrl);
>> -    wait_for_completion_timeout(&ctrl->enumeration,
>> -                    msecs_to_jiffies(TIMEOUT_MS));
>>       ret = qcom_swrm_register_dais(ctrl);
>>       if (ret)
>>           goto err_master_add;

Re: [PATCH v2] soundwire: qcom: remove redundant wait for completion

[Srinivas Kandagatla]

On 25/02/2022 15:43, Pierre-Louis Bossart wrote:
> 
> 
> On 2/25/22 08:45, Srinivas Kandagatla wrote:
>>
>>
>> On 25/02/2022 14:44, Srinivasa Rao Mandadapu wrote:
>>> Remove wait_for_completion_timeout from soundwire probe as it seems
>>> unnecessary and device enumeration is anyway not happening here,
>>> hence this api is blocking till it completes max wait time.
>>> Also, as device enumeration event is dependent on wcd938x probe to be
>>> completed, its of no use waiting here.
>>> Waiting here increasing the boot time almost 4 seconds and impacting
>>> other modules like touch screen.
>>>
>>> Fixes: 06dd96738d618 ("soundwire: qcom: wait for enumeration to be
>>> complete in probe")
>>>
>>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>>> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>>> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>>
>> LGTM,
>>
>> Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> 
> I don't get the idea, sorry.
> 
> If you look at the code, these are the cases where this 'struct
> completion' is used
> 
> 	struct completion enumeration;
> 	complete(&ctrl->enumeration);
> 	/* Enable Auto enumeration */
> 	init_completion(&ctrl->enumeration);
> 	wait_for_completion_timeout(&ctrl->enumeration,
> 
> 
> so if you remove the wait_for_completeion, then you might just as well
> remove the whole thing and revert 06dd96738d618
> 
> what am I missing?
Yes, that makes more sense to revert it, as it is the only user of this 
completion.
We could add it back when we really need this again in pm runtime setup 
if required.

--srini
> 
> 
>>> ---
>>>    drivers/soundwire/qcom.c | 2 --
>>>    1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
>>> index 5481341..9a32a24 100644
>>> --- a/drivers/soundwire/qcom.c
>>> +++ b/drivers/soundwire/qcom.c
>>> @@ -1309,8 +1309,6 @@ static int qcom_swrm_probe(struct
>>> platform_device *pdev)
>>>        }
>>>          qcom_swrm_init(ctrl);
>>> -    wait_for_completion_timeout(&ctrl->enumeration,
>>> -                    msecs_to_jiffies(TIMEOUT_MS));
>>>        ret = qcom_swrm_register_dais(ctrl);
>>>        if (ret)
>>>            goto err_master_add;

Re: [PATCH v2] soundwire: qcom: remove redundant wait for completion

[Srinivasa Rao Mandadapu]

On 2/25/2022 9:13 PM, Pierre-Louis Bossart wrote:
Thanks for your time Bossart!!!
>
> On 2/25/22 08:45, Srinivas Kandagatla wrote:
>>
>> On 25/02/2022 14:44, Srinivasa Rao Mandadapu wrote:
>>> Remove wait_for_completion_timeout from soundwire probe as it seems
>>> unnecessary and device enumeration is anyway not happening here,
>>> hence this api is blocking till it completes max wait time.
>>> Also, as device enumeration event is dependent on wcd938x probe to be
>>> completed, its of no use waiting here.
>>> Waiting here increasing the boot time almost 4 seconds and impacting
>>> other modules like touch screen.
>>>
>>> Fixes: 06dd96738d618 ("soundwire: qcom: wait for enumeration to be
>>> complete in probe")
>>>
>>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>>> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>>> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> LGTM,
>>
>> Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> I don't get the idea, sorry.
>
> If you look at the code, these are the cases where this 'struct
> completion' is used
>
> 	struct completion enumeration;
> 	complete(&ctrl->enumeration);
> 	/* Enable Auto enumeration */
> 	init_completion(&ctrl->enumeration);
> 	wait_for_completion_timeout(&ctrl->enumeration,
>
>
> so if you remove the wait_for_completeion, then you might just as well
> remove the whole thing and revert 06dd96738d618
>
> what am I missing?
Okay. Sent new patch with reverting commit 06dd96738d618
>
>
>>> ---
>>>    drivers/soundwire/qcom.c | 2 --
>>>    1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
>>> index 5481341..9a32a24 100644
>>> --- a/drivers/soundwire/qcom.c
>>> +++ b/drivers/soundwire/qcom.c
>>> @@ -1309,8 +1309,6 @@ static int qcom_swrm_probe(struct
>>> platform_device *pdev)
>>>        }
>>>          qcom_swrm_init(ctrl);
>>> -    wait_for_completion_timeout(&ctrl->enumeration,
>>> -                    msecs_to_jiffies(TIMEOUT_MS));
>>>        ret = qcom_swrm_register_dais(ctrl);
>>>        if (ret)
>>>            goto err_master_add;

Re: [PATCH v2] soundwire: qcom: remove redundant wait for completion

[Srinivasa Rao Mandadapu]

On 2/25/2022 11:33 PM, Srinivas Kandagatla wrote:
Thanks for Your time Srini!!!
>
> On 25/02/2022 15:43, Pierre-Louis Bossart wrote:
>>
>>
>> On 2/25/22 08:45, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 25/02/2022 14:44, Srinivasa Rao Mandadapu wrote:
>>>> Remove wait_for_completion_timeout from soundwire probe as it seems
>>>> unnecessary and device enumeration is anyway not happening here,
>>>> hence this api is blocking till it completes max wait time.
>>>> Also, as device enumeration event is dependent on wcd938x probe to be
>>>> completed, its of no use waiting here.
>>>> Waiting here increasing the boot time almost 4 seconds and impacting
>>>> other modules like touch screen.
>>>>
>>>> Fixes: 06dd96738d618 ("soundwire: qcom: wait for enumeration to be
>>>> complete in probe")
>>>>
>>>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>>>> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>>>> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>>>
>>> LGTM,
>>>
>>> Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> I don't get the idea, sorry.
>>
>> If you look at the code, these are the cases where this 'struct
>> completion' is used
>>
>>     struct completion enumeration;
>>     complete(&ctrl->enumeration);
>>     /* Enable Auto enumeration */
>>     init_completion(&ctrl->enumeration);
>>     wait_for_completion_timeout(&ctrl->enumeration,
>>
>>
>> so if you remove the wait_for_completeion, then you might just as well
>> remove the whole thing and revert 06dd96738d618
>>
>> what am I missing?
> Yes, that makes more sense to revert it, as it is the only user of 
> this completion.
> We could add it back when we really need this again in pm runtime 
> setup if required.
>
> --srini
Thanks for confirmation. Sent revert patch.
>>
>>
>>>> ---
>>>>    drivers/soundwire/qcom.c | 2 --
>>>>    1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
>>>> index 5481341..9a32a24 100644
>>>> --- a/drivers/soundwire/qcom.c
>>>> +++ b/drivers/soundwire/qcom.c
>>>> @@ -1309,8 +1309,6 @@ static int qcom_swrm_probe(struct
>>>> platform_device *pdev)
>>>>        }
>>>>          qcom_swrm_init(ctrl);
>>>> -    wait_for_completion_timeout(&ctrl->enumeration,
>>>> -                    msecs_to_jiffies(TIMEOUT_MS));
>>>>        ret = qcom_swrm_register_dais(ctrl);
>>>>        if (ret)
>>>>            goto err_master_add;