Message-ID: <aa564260-61c2-e6ee-4b86-4689b84bfc02@foss.st.com>
Date:   Fri, 10 Sep 2021 18:39:52 +0200
From:   Arnaud POULIQUEN <arnaud.pouliquen@...s.st.com>
To:     Bjorn Andersson <bjorn.andersson@...aro.org>
CC:     Ohad Ben-Cohen <ohad@...ery.com>,
        <linux-remoteproc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Dong Aisheng <aisheng.dong@....com>,
        "Konrad Dybcio" <konrad.dybcio@...ainline.org>,
        Alex Elder <elder@...aro.org>,
        "Linus Torvalds" <torvalds@...ux-foundation.org>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Loic PALLARDY <loic.pallardy@...com>
Subject: Re: [GIT PULL] remoteproc updates for v5.15



On 9/10/21 4:59 PM, Bjorn Andersson wrote:
> On Fri 10 Sep 06:32 PDT 2021, Arnaud POULIQUEN wrote:
> 
>> Hello Bjorn,
>>
> 
> Good morning Arnaud,
> 
>>
>> On 9/7/21 4:00 PM, Bjorn Andersson wrote:
>>> The following changes since commit e73f0f0ee7541171d89f2e2491130c7771ba58d3:
>>>
>>>   Linux 5.14-rc1 (2021-07-11 15:07:40 -0700)
>>>
>>> are available in the Git repository at:
>>>
>>>   https://git.kernel.org/pub/scm/linux/kernel/git/andersson/remoteproc.git tags/rproc-v5.15
>>>
>>> for you to fetch changes up to a0a77028c85ad1f6f36c3ceea21b30dc43721665:
>>>
>>>   remoteproc: q6v5_pas: Add sdm660 ADSP PIL compatible (2021-08-04 12:37:32 -0500)
>>>
>>> ----------------------------------------------------------------
>>> remoteproc updates for v5.15
>>>
>>
>> I was expecting to see a pull request for the RPMsg framework as well,
>> integrating my work around the rpmsg_char driver restructuring.
>>
>> These series listed at the end of my mail have been reviewed by Mathieu Poirier
>> (RPMsg framework co-maintainer) before the 5.14 pull requests. Then on July 12,
>> I re-based the series on v14.1-rc1 expecting to give you enough time to
>> integrate them for the next 5.15 merge window.
>>
> 
> Yes, I definitely had enough time.
> 
>> Could you please tell me if it is just a miss or if you have some concerns on
>> them? Because I never received feedback from you for this work.
>>
> 
> I did see that you and Mathieu had reached an agreement on the patches
> and set out to apply the patches.
> 
> But as I look at the patches I realize that you're refactoring the
> entire design of how rpmsg_char works and last time we spoke about the
> existing users I got the feeling that you had no way to validate that
> they still work after this refactoring. And in those discussions I
> highlighted a few things that would break existing users.
> 
> So I felt the need to convince myself that your series does indeed not
> break existing users.

I understand, Mathieu and I have kept in mind this compatibility. I think we
maintain the legacy. But yes, agree with you that a non regression test using
the Qcom drivers is important to ensure that.

> 
> Unfortunately I dropped the ball on getting back to do this.
> 
>> Or maybe I missed something in the process, I thought that Matthieu's
>> "reviewed-by" was sufficient to be accepted.
>>
> 
> The change is complex, affects existing users, it introduces new ABI and
> that I don't believe Mathieu has the means of testing the existing
> users(?). So while I trust Mathieu's R-b, I did want to take one more
> look at it.
> 
>> How could we move forward on this work, which also seems to interest some other
>> companies?
>>
> 
> I'll make sure to carve out the necessary time in the coming days to go
> through the patches and let's take it from there.
> 
>> Related series:
>> - [PATCH v5 0/4] Restructure the rpmsg char to decorrelate the control part
>>   https://lkml.org/lkml/2021/7/12/2872
>> - [PATCH v4 0/4] rpmsg: char: introduce the rpmsg-raw channel???
>>   https://lkml.org/lkml/2021/7/12/2908
>> - [PATCH v3] rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
>>   https://lkml.org/lkml/2021/7/12/2913
>>
> 
> You have 9 patches over 3 different series in different versions, where
> things certainly depend on each other.
> 
> I believe I asked you if we could do this step-wise, I didn't mean that
> we should split it in multiple steps that needs to be taken at the same
> time...

This split is coming during review discussions with Mathieu, to ease the
reviews. So versions depend on series reviews iteration.
But it is not necessary to get them in one package "just" to respect dependencies.

Here is the order you have to respect:

1) [PATCH v5 0/4] Restructure the rpmsg char to decorrelate the control part
=> first patch that impact rpmsg_char client drivers.
minor impact but right, not tested on Qcom platform (only build testd for Qcom
drivers)

2) [PATCH v4 0/4] rpmsg: char: introduce the rpmsg-raw channel
As described in the cover letter to be applied on top of:
[PATCH v5 0/4] Restructure the rpmsg char to decorrelate the control part
=> tested by  Julien Massot from iot.bzh

3) [PATCH v3] rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
To apply on top of the two previous ones.

So either you can integrate and test them one by one, or i can squash them in a
new series if this can simplify your integration.

In any cases, just send me an email if you need patch-sets refactoring, or
clarification on some points.


>> - [PATCH v2] rpmsg: Fix rpmsg_create_ept return when RPMSG config is not defined
>>   https://lkml.org/lkml/2021/7/12/2877
> 
> Then you have this, which I thought was related to the others when I
> browsed through the patch list, and therefor didn't merge. But now that
> I look again I see that this is unrelated.
> 
> I've added the appropriate Fixes tag and picked this up now.

Correct independent patch, thanks for the patch update.

Thanks,
Arnaud

> 
> Regards,
> Bjorn
> 
>> Thanks in advance,
>> Regards,
>> Arnaud
>>
>>
>>> This moves the crash recovery worker to the freezable work queue to
>>> avoid interaction with other drivers during suspend & resume. It fixes a
>>> couple of typos in comments.
>>>
>>> It adds support for handling the audio DSP on SDM660 and it fixes a race
>>> between the Qualcomm wireless subsystem driver and the associated driver
>>> for the RF chip.
>>>
>>> ----------------------------------------------------------------
>>> Alex Elder (1):
>>>       remoteproc: use freezable workqueue for crash notifications
>>>
>>> Bjorn Andersson (1):
>>>       remoteproc: qcom: wcnss: Fix race with iris probe
>>>
>>> Dong Aisheng (2):
>>>       remoteproc: fix an typo in fw_elf_get_class code comments
>>>       remoteproc: fix kernel doc for struct rproc_ops
>>>
>>> Konrad Dybcio (2):
>>>       dt-bindings: remoteproc: qcom: adsp: Add SDM660 ADSP
>>>       remoteproc: q6v5_pas: Add sdm660 ADSP PIL compatible
>>>
>>>  .../devicetree/bindings/remoteproc/qcom,adsp.yaml  |   1 +
>>>  drivers/remoteproc/qcom_q6v5_pas.c                 |   1 +
>>>  drivers/remoteproc/qcom_wcnss.c                    |  49 +++------
>>>  drivers/remoteproc/qcom_wcnss.h                    |   4 +-
>>>  drivers/remoteproc/qcom_wcnss_iris.c               | 120 +++++++++++++--------
>>>  drivers/remoteproc/remoteproc_core.c               |   4 +-
>>>  drivers/remoteproc/remoteproc_elf_helpers.h        |   2 +-
>>>  include/linux/remoteproc.h                         |   5 +-
>>>  8 files changed, 96 insertions(+), 90 deletions(-)
>>>

Powered by blists - more mailing lists