Date:   Fri, 10 Sep 2021 07:59:17 -0700
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     Arnaud POULIQUEN <arnaud.pouliquen@...s.st.com>
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 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.

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...

> - [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.

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