devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Doug Anderson <dianders@chromium.org>
Cc: Andy Gross <andy.gross@linaro.org>,
	David Brown <david.brown@linaro.org>,
	Sibi Sankar <sibis@codeaurora.org>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	devicetree@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 03/10] arm64: dts: sdm845: Introduce ADSP and CDSP PAS nodes
Date: Tue, 22 Jan 2019 16:26:10 -0800	[thread overview]
Message-ID: <20190123002610.GE31919@minitux> (raw)
In-Reply-To: <CAD=FV=WXind-uk7V_XTDgM83LZbmtKb2Ln6pQjmUkaZMz8SFsA@mail.gmail.com>

On Tue 22 Jan 15:46 PST 2019, Doug Anderson wrote:

> Hi,
> 
> On Mon, Jan 21, 2019 at 9:52 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > Add the ADSP and CDSP nodes for PAS-based remoteproc, supporting booting
> > these cores on e.g. the MTP, and enable the same for the MTP.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >
> > Changes since v2:
> > - New patch
> >
> >  arch/arm64/boot/dts/qcom/sdm845-mtp.dts |  8 ++++
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi    | 58 +++++++++++++++++++++++++
> >  2 files changed, 66 insertions(+)
> 
> It's a bit of a nit of mine that if it's not totally obvious what
> acronyms mean that they should be spelled out in places that use them.
> 
> In this case I believe ADSP is the Audio DSP.  Is CDSP the Camera DSP?  ...or ?
> 

C as in Compute. I'll spell these out as I respin the series.

> 
> > +       adsp_pas: remoteproc-adsp {
> > +               compatible = "qcom,sdm845-adsp-pas";
> > +
> > +               interrupts-extended = <&intc GIC_SPI 162 IRQ_TYPE_EDGE_RISING>,
> > +                                     <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
> > +                                     <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
> > +                                     <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
> > +                                     <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>;
> > +               interrupt-names = "wdog", "fatal", "ready",
> > +                                 "handover", "stop-ack";
> > +
> > +               clocks = <&xo_board>;
> > +               clock-names = "xo";
> 
> I've found that nearly all the places that refer to xo_board are wrong
> and should actually point to '<&rpmhcc RPMH_CXO_CLK>'.  Maybe yours
> should too?
> 

Yes, xo_board is a fake clock representing the 19.2MHz clock feeding the
cxo (or cxo2) pad of the SoC. So you're definitely right in that this
should be referencing the actual 19.2MHz clock.

We've kept referring to this as xo_board, as we don't handle probe
deferral when gcc will probe earlier than rpmcc in the boot and for
other non-clock drivers the fear of actually hitting 0 on the refcounter
for this (you don't want to disable the cxo while running the system).

I'll give it a spin with appropriate reference and see what happens, I
think this should either be changed or documented in the commit message.

Thanks,
Bjorn

  reply	other threads:[~2019-01-23  0:26 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-22  5:51 [PATCH v3 00/10] Qualcomm AOSS QMP driver and modem dts Bjorn Andersson
2019-01-22  5:51 ` [PATCH v3 01/10] arm64: dts: qcom: sdm845: Update PIL region memory map Bjorn Andersson
2019-01-22 18:58   ` Stephen Boyd
2019-01-22 19:24     ` Bjorn Andersson
2019-01-22 23:10       ` Doug Anderson
2019-01-23  0:30         ` Bjorn Andersson
2019-01-22 23:16   ` Doug Anderson
2019-01-23  0:39     ` Bjorn Andersson
2019-01-25 17:40   ` Sibi Sankar
2019-01-22  5:51 ` [PATCH v3 02/10] arm64: dts: qcom: sdm845: Define rmtfs memory Bjorn Andersson
2019-01-22 23:26   ` Doug Anderson
2019-01-22 23:34     ` Brian Norris
2019-01-23  0:47     ` Bjorn Andersson
2019-01-22  5:51 ` [PATCH v3 03/10] arm64: dts: sdm845: Introduce ADSP and CDSP PAS nodes Bjorn Andersson
2019-01-22 23:46   ` Doug Anderson
2019-01-23  0:26     ` Bjorn Andersson [this message]
2019-01-23  0:40       ` Doug Anderson
2019-01-23  1:09         ` Bjorn Andersson
2019-01-23 23:24           ` Doug Anderson
2019-01-24  6:22             ` Stephen Boyd
2019-01-22  5:51 ` [PATCH v3 04/10] dt-bindings: soc: qcom: Add AOSS QMP binding Bjorn Andersson
2019-01-22 19:04   ` Stephen Boyd
2019-01-22 19:25     ` Bjorn Andersson
2019-01-22 19:28       ` Stephen Boyd
2019-01-22  5:51 ` [PATCH v3 05/10] soc: qcom: Add AOSS QMP communication driver Bjorn Andersson
2019-01-22  5:51 ` [PATCH v3 06/10] soc: qcom: Add AOSS QMP genpd provider Bjorn Andersson
2019-01-22  5:51 ` [PATCH v3 07/10] remoteproc: q6v5-mss: Vote for rpmh power domains Bjorn Andersson
2019-01-23  0:01   ` Doug Anderson
2019-01-22  5:51 ` [PATCH v3 08/10] remoteproc: q6v5-mss: Active powerdomain for SDM845 Bjorn Andersson
2019-01-22  5:51 ` [PATCH v3 09/10] arm64: dts: qcom: Add AOSS QMP node Bjorn Andersson
2019-01-22  5:51 ` [PATCH v3 10/10] arm64: dts: qcom: sdm845: Add Q6V5 MSS node Bjorn Andersson
2019-01-23  0:28   ` Doug Anderson
2019-01-23  1:10     ` Bjorn Andersson

Reply instructions:

You may reply publicly 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=20190123002610.GE31919@minitux \
    --to=bjorn.andersson@linaro.org \
    --cc=andy.gross@linaro.org \
    --cc=david.brown@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=sibis@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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).