All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <swboyd@chromium.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Maulik Shah <mkshah@codeaurora.org>,
	evgreen@chromium.org, mka@chromium.org,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	agross@kernel.org, dianders@chromium.org, linux@roeck-us.net,
	rnayak@codeaurora.org, lsrao@codeaurora.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH v8 3/5] arm64: dts: qcom: sc7180: Enable SoC sleep stats
Date: Fri, 4 Jun 2021 21:53:54 +0000	[thread overview]
Message-ID: <CAE-0n511_GHcyPDSeDaf5QSqVQqyHOqxJCGaSWNr=x9uotegLg@mail.gmail.com> (raw)
In-Reply-To: <YLhCGC/qgP6ESNl7@yoga>

Quoting Bjorn Andersson (2021-06-02 19:44:40)
> On Wed 02 Jun 19:26 CDT 2021, Stephen Boyd wrote:
>
> > Quoting Bjorn Andersson (2021-05-31 10:57:03)
> > > On Wed 26 May 18:30 CDT 2021, Stephen Boyd wrote:
> > >
> > > > Quoting Maulik Shah (2021-05-21 04:26:09)
> > > > > @@ -3223,6 +3223,11 @@
> > > > >                         #power-domain-cells = <1>;
> > > > >                 };
> > > > >
> > > > > +               rpmh-sleep-stats@c3f0000 {
> > > > > +                       compatible = "qcom,rpmh-sleep-stats";
> > > > > +                       reg = <0 0x0c3f0000 0 0x400>;
> > > > > +               };
> > > > > +
> > > >
> > > > Does this need to be in DT? Can the sc7180-aoss-qmp driver use the
> > > > aux-bus and stick the sleep stats device on there?
> > > >
> > >
> > > The AOSS memory space has N chunks of "message ram", one is used for the
> > > QMP protocol (presumably the APSS specific one), a different one is used
> > > for the sleep stats.
> > >
> > > I presume we could have come up with a binding for the entire AOSS/AOP
> > > and then describe (either implicit or explicitly) the QMP and
> > > debug-stats under that.
> > >
> > > But we'd also have to come up with the same container-device for the RPM
> > > case.
> >
> > Because the rpm node doesn't include this region of memory today? I
> > still fail to see why we're changing the existing binding and adding a
> > DT node for this new region that is basically a debug feature.
>
> We're not changing the binding, the memory region for the "AOSS QMP"
> thing was never larger than 0x400.
>
> 0x100000 is the size of all the AOSS "msg_ram" regions. We don't have
> this whole thing described in a binding and we don't have an
> implementation for the whole thing.
>
> If we're going for that we'd need to extend the binding to indicate
> which of the msg_ram regions are used for APSS QMP and for debug stats
> on particular platform (either by compatible, explicit properties or as
> some subnodes).

Fair enough. At the least, can we change the name of the node then to
'sram' or 'ram'? The 'rpmh-sleep-stats' node name is nonsense.

>
>
> That said, as I looked into my other objection, for the RPM
> (non-hardened) case it seems that we're actually describing the RPM
> region. So there it would make sense to describe it as such in DT - but
> we don't have any other code (that I'm aware of) that would implement
> the "qcom,<platform>-rpm".
>

I only half parsed this part. Are you saying that because we don't have
a driver for qcom,<platform>-rpm we shouldn't keep it all within the rpm
node?

  reply	other threads:[~2021-06-04 21:54 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-21 11:26 [PATCH v8 0/5] Introduce SoC sleep stats driver Maulik Shah
2021-05-21 11:26 ` [PATCH v8 1/5] dt-bindings: Introduce SoC sleep stats bindings Maulik Shah
2021-05-21 11:26 ` [PATCH v8 2/5] soc: qcom: Add SoC sleep stats driver Maulik Shah
2021-05-26 23:45   ` Stephen Boyd
2021-09-05 12:59     ` Maulik Shah
2021-05-31 17:30   ` Bjorn Andersson
2021-05-21 11:26 ` [PATCH v8 3/5] arm64: dts: qcom: sc7180: Enable SoC sleep stats Maulik Shah
2021-05-26 23:30   ` Stephen Boyd
2021-05-31 17:57     ` Bjorn Andersson
2021-06-03  0:26       ` Stephen Boyd
2021-06-03  2:44         ` Bjorn Andersson
2021-06-04 21:53           ` Stephen Boyd [this message]
2021-06-06  3:42             ` Bjorn Andersson
2021-09-05 12:59               ` Maulik Shah
2021-05-21 11:26 ` [PATCH v8 4/5] arm64: defconfig: Enable SoC sleep stats driver Maulik Shah
2021-05-26 23:30   ` Stephen Boyd
2021-05-21 11:26 ` [PATCH v8 5/5] arm64: dts: qcom: sc7280: Enable SoC sleep stats Maulik Shah
2021-05-26 23:30   ` Stephen Boyd

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='CAE-0n511_GHcyPDSeDaf5QSqVQqyHOqxJCGaSWNr=x9uotegLg@mail.gmail.com' \
    --to=swboyd@chromium.org \
    --cc=agross@kernel.org \
    --cc=bjorn.andersson@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@roeck-us.net \
    --cc=lsrao@codeaurora.org \
    --cc=mka@chromium.org \
    --cc=mkshah@codeaurora.org \
    --cc=rnayak@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.