On 3 July 2022 13:13:51 GMT+03:00, Stephan Gerhold <stephan@xxxxxxxxxxx> wrote: >On Sat, May 14, 2022 at 03:01:07AM +0300, Dmitry Baryshkov wrote: >> From: Konrad Dybcio <konradybcio@xxxxxxxxx> >> >> This adds support for sdm630/636/660 modem subsystem >> remote processor. >> >> Signed-off-by: Konrad Dybcio <konradybcio@xxxxxxxxx> >> [DB: fixed commit message, removed note about modem restarting regularly] >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > >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
