Re: [PATCH 3/4] remoteproc: qcom: q6v5-mss: Add support for SDM630/636/660 MSS remoteproc

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On 3 July 2022 13:13:51 GMT+03:00, Stephan Gerhold <stephan@gerhold.net> wrote:
>On Sat, May 14, 2022 at 03:01:07AM +0300, Dmitry Baryshkov wrote:
>> From: Konrad Dybcio <konradybcio@gmail.com>
>> 
>> This adds support for sdm630/636/660 modem subsystem
>> remote processor.
>> 
>> Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
>> [DB: fixed commit message, removed note about modem restarting regularly]
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
>I was only looking at this by coincidence recently but since it doesn't
>seem to be applied yet(?) some comments below.

Thanks for your comments, I will take a look next week.

>
>> ---
>>  drivers/remoteproc/qcom_q6v5_mss.c | 111 +++++++++++++++++++++++++++++
>>  1 file changed, 111 insertions(+)
>> 
>> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
>> index af217de75e4d..7a4cca30db8a 100644
>> --- a/drivers/remoteproc/qcom_q6v5_mss.c
>> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
>[...]
>> +		}
>> +
>>  		/* Remove word line clamp */
>>  		val = readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>>  		val &= ~QDSP6v56_CLAMP_WL;
>
>All in all this looks almost exactly like the existing code for
>MSS_MSM8996/8. Wouldn't it be cleaner to just add an if statement for
>the QDSP6V62SS_BHS_STATUS readl_poll_timeout() to the existing code?

For quite some time I was unhappy with this part of the mss driver. Maybe it's time to review platform-specific code in attempt to generalise it.

-- 
With best wishes
Dmitry