diff mbox series

[v3,02/10] arm64: dts: qcom: sdm845: Define rmtfs memory

Message ID 20190122055112.30943-3-bjorn.andersson@linaro.org (mailing list archive)
State New, archived
Headers show
Series Qualcomm AOSS QMP driver and modem dts | expand

Commit Message

Bjorn Andersson Jan. 22, 2019, 5:51 a.m. UTC
Define the rmtfs memory node, as described in version 10 of the memory
map.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- New patch

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Doug Anderson Jan. 22, 2019, 11:26 p.m. UTC | #1
Hi,

On Mon, Jan 21, 2019 at 9:51 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> Define the rmtfs memory node, as described in version 10 of the memory
> map.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v2:
> - New patch
>
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index cdcac3704c13..64f57cc5c61a 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -72,6 +72,15 @@
>                 #size-cells = <2>;
>                 ranges;
>
> +               rmtfs@85d00000 {
> +                       compatible = "qcom,rmtfs-mem";
> +                       reg = <0 0x85d00000 0 0x200000>;
> +                       no-map;
> +
> +                       qcom,client-id = <1>;
> +                       qcom,vmid = <15>;
> +               };

Ah, I saw this after I posted my comments to patch #1.  I guess this
is the same as this node we have in our cheza board file downstream
(need to get that posted upstream soon):

rmtfs@88f00000 {
  compatible = "qcom,rmtfs-mem";
  reg = <0x0 0x88f00000 0x0 0x800000>;
  no-map;

  qcom,client-id = <1>;
};

That brings up a few things:

1. You should add a node label here.  This allows us to act on the
node more easily from board files, like setting it to disabled or
changing it.

2. In https://crrev.com/c/1119572, the argument was made that the size
of this carveout is board-specific.  That makes it hard to put it in
sdm845.dts.


-Doug
Brian Norris Jan. 22, 2019, 11:34 p.m. UTC | #2
On Tue, Jan 22, 2019 at 3:26 PM Doug Anderson <dianders@chromium.org> wrote:
> 2. In https://crrev.com/c/1119572, the argument was made that the size
> of this carveout is board-specific.  That makes it hard to put it in
> sdm845.dts.

And we've increased the size of the memory carve-out since then,
because the memory requirements depend on the SW features you're using
on the modem side.

So that's definitely a reason for giving it a label we can modify it
with, at a minimum (as Doug suggested). I'm somewhat ambivalent as to
whether this should or shouldn't go in the base sdm845.dtsi -- it's
still a feature where most of the definitions (e.g., compatible;
client ID) should be applicable to all boards.

Brian
Bjorn Andersson Jan. 23, 2019, 12:47 a.m. UTC | #3
On Tue 22 Jan 15:26 PST 2019, Doug Anderson wrote:

> Hi,
> 
> On Mon, Jan 21, 2019 at 9:51 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > Define the rmtfs memory node, as described in version 10 of the memory
> > map.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >
> > Changes since v2:
> > - New patch
> >
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > index cdcac3704c13..64f57cc5c61a 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > @@ -72,6 +72,15 @@
> >                 #size-cells = <2>;
> >                 ranges;
> >
> > +               rmtfs@85d00000 {
> > +                       compatible = "qcom,rmtfs-mem";
> > +                       reg = <0 0x85d00000 0 0x200000>;
> > +                       no-map;
> > +
> > +                       qcom,client-id = <1>;
> > +                       qcom,vmid = <15>;
> > +               };
> 
> Ah, I saw this after I posted my comments to patch #1.  I guess this
> is the same as this node we have in our cheza board file downstream
> (need to get that posted upstream soon):
> 
> rmtfs@88f00000 {
>   compatible = "qcom,rmtfs-mem";
>   reg = <0x0 0x88f00000 0x0 0x800000>;
>   no-map;
> 
>   qcom,client-id = <1>;
> };
> 
> That brings up a few things:
> 
> 1. You should add a node label here.  This allows us to act on the
> node more easily from board files, like setting it to disabled or
> changing it.
> 

I'll make sure to label it.

> 2. In https://crrev.com/c/1119572, the argument was made that the size
> of this carveout is board-specific.  That makes it hard to put it in
> sdm845.dts.
> 

I don't think I've seen a modern platform where this isn't 2MB, so I
think it's safe to add it to the platform. But a label sounds good, if
someone out there has a custom modem firmware with some odd changes in
this area.

I'll label it, to make it possible to move, resize or reclaim in boards.

Regards,
Bjorn
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index cdcac3704c13..64f57cc5c61a 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -72,6 +72,15 @@ 
 		#size-cells = <2>;
 		ranges;
 
+		rmtfs@85d00000 {
+			compatible = "qcom,rmtfs-mem";
+			reg = <0 0x85d00000 0 0x200000>;
+			no-map;
+
+			qcom,client-id = <1>;
+			qcom,vmid = <15>;
+		};
+
 		memory@85fc0000 {
 			reg = <0 0x85fc0000 0 0x20000>;
 			no-map;