LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Doug Anderson <dianders@chromium.org>
Cc: Sibi Sankar <sibis@codeaurora.org>,
	Rob Herring <robh+dt@kernel.org>,
	Andy Gross <andy.gross@linaro.org>,
	David Brown <david.brown@linaro.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	"open list:ARM/QUALCOMM SUPPORT" <linux-soc@vger.kernel.org>,
	devicetree@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
	tsoni@codeaurora.org, clew@codeaurora.org,
	akdwived@codeaurora.org, Mark Rutland <mark.rutland@arm.com>,
	linux-remoteproc@vger.kernel.org,
	Evan Green <evgreen@chromium.org>,
	Brian Norris <briannorris@chromium.org>,
	sricharan@codeaurora.org
Subject: Re: [PATCH v5 8/8] arm64: dts: qcom: sdm845: Add Q6V5 MSS node
Date: Sat, 12 Jan 2019 10:42:55 -0800
Message-ID: <20190112184255.GA28907@builder> (raw)
In-Reply-To: <CAD=FV=Xu3m5=ByHo_38T=H2cm4VqkndVjJ1rUkD2gp-2SWFXdQ@mail.gmail.com>

On Fri 11 Jan 13:06 PST 2019, Doug Anderson wrote:

> Hi,
> 
> On Wed, Jan 9, 2019 at 9:00 AM Sibi Sankar <sibis@codeaurora.org> wrote:
> >
> > This patch adds Q6V5 MSS remoteproc node for SDM845 SoCs.
> >
> > Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > ---
> >
> > v5:
> >   * Use qmp_aop updated dt binding
> 
> nit: since this is now a singleton patch in v5 (because patches #1 -
> #7 landed), the general policy is to drop the "8/8" in the subject.
> AKA I believe the subject of the patch ought to have been:
> 
> [PATCH v5] arm64: dts: qcom: sdm845: Add Q6V5 MSS node
> 
> 
> > v3:
> >   * with shutdown-ack irq redesign make it mandatory,
> >     merge multiple patches into a single one
> >
> > v2:
> >   * Fixed style changes
> >   * Added missing clocks in the dt-bindings
> >   * Split mss remoteproc node into a number of patches
> >
> > This patch depends on the following bindings:
> > https://patchwork.kernel.org/patch/10662089/ - mba/mpss reserved regions
> > https://patchwork.kernel.org/patch/10657325/ - pdc reset node
> > https://patchwork.kernel.org/patch/10753659/ - rpmhpd dt node
> > https://patchwork.kernel.org/patch/10749469/ - AOP QMP dt bindings
> > https://patchwork.kernel.org/patch/10751757/ - shutdown-irq binding
> >
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 60 ++++++++++++++++++++++++++++
> >  1 file changed, 60 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > index 5da9fa1feb8a..e021b15f87fd 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > @@ -1366,6 +1366,66 @@
> >                         };
> >                 };
> >
> > +               remoteproc@4080000 {
> 
> It would be handy if you added a label here, AKA:
> 
> mss_pil: remoteproc@4080000 {
> 
> It's expected that boards will need to refer to this node so that they
> can provide a firmware-name, so you need to give them a label to grab
> onto.
> 

I agree.

> ...and actually, I wonder if boards will also need to be able to set
> status = "okay"?  Right now they don't because you don't have a status
> = "disabled" in sdm845.dtsi, but maybe you should?  Are there ever
> going to be any boards with sdm845 that don't hook up the modem?
> 

The modem is status "ok" on my SDA845 device (but haven't verified
upstream myself yet), so I think it's fine to leave it enabled.

But merging this as is will cause the modem to crash repeatedly until
rmtfs is present. Sibi is making progress on tying this to rmtfs, which
would be accompanied by the following commit:

https://lore.kernel.org/lkml/20180524192141.20323-1-ramon.fried@gmail.com/

I'm okay with merging that now, to unblock the merge of this patch.
Would appreciate an Ack on this.


Also, Sibi, the glink-edge binding doesn't define the mbox-names
property, so please drop it.

Regards,
Bjorn

      reply index

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-09 17:00 Sibi Sankar
2019-01-11 21:06 ` Doug Anderson
2019-01-12 18:42   ` Bjorn Andersson [this message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190112184255.GA28907@builder \
    --to=bjorn.andersson@linaro.org \
    --cc=akdwived@codeaurora.org \
    --cc=andy.gross@linaro.org \
    --cc=briannorris@chromium.org \
    --cc=clew@codeaurora.org \
    --cc=david.brown@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=evgreen@chromium.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=linux-soc@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=sibis@codeaurora.org \
    --cc=sricharan@codeaurora.org \
    --cc=tsoni@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox