| Message ID | 20190122055112.30943-2-bjorn.andersson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Qualcomm AOSS QMP driver and modem dts | expand |
Quoting Bjorn Andersson (2019-01-21 21:51:03) > @@ -103,10 +138,30 @@ > no-map; > }; > > + venus_mem: memory@95800000 { > + reg = <0 0x95800000 0 0x500000>; > + no-map; > + }; > + > + cdsp_mem: memory@95d00000 { > + reg = <0 0x95d00000 0 0x800000>; > + no-map; > + }; > + > mba_region: memory@96500000 { > reg = <0 0x96500000 0 0x200000>; > no-map; > }; > + > + slpi_mem: memory@96700000 { > + reg = <0 0x96700000 0 0x1400000>; > + no-map; > + }; > + > + spss_mem: memory@97b00000 { > + reg = <0 0x97b00000 0 0x100000>; > + no-map; > + }; > }; > What's the plan if certain configurations don't use all these carveouts? Can we mark the reservation nodes as status = "disabled", or the reverse and mark them as status = "ok" in all boards, and then reclaim the memory for peripherals we don't care to use?
On Tue 22 Jan 10:58 PST 2019, Stephen Boyd wrote: > Quoting Bjorn Andersson (2019-01-21 21:51:03) > > @@ -103,10 +138,30 @@ > > no-map; > > }; > > > > + venus_mem: memory@95800000 { > > + reg = <0 0x95800000 0 0x500000>; > > + no-map; > > + }; > > + > > + cdsp_mem: memory@95d00000 { > > + reg = <0 0x95d00000 0 0x800000>; > > + no-map; > > + }; > > + > > mba_region: memory@96500000 { > > reg = <0 0x96500000 0 0x200000>; > > no-map; > > }; > > + > > + slpi_mem: memory@96700000 { > > + reg = <0 0x96700000 0 0x1400000>; > > + no-map; > > + }; > > + > > + spss_mem: memory@97b00000 { > > + reg = <0 0x97b00000 0 0x100000>; > > + no-map; > > + }; > > }; > > > > What's the plan if certain configurations don't use all these carveouts? > Can we mark the reservation nodes as status = "disabled", or the reverse > and mark them as status = "ok" in all boards, and then reclaim the > memory for peripherals we don't care to use? > The code path that picks these up does look for "status", so I suggest that we leave them all enabled in the platform dtsi and then let the device's reclaim them as needed. Regards, Bjorn
Hi, On Tue, Jan 22, 2019 at 11:24 AM Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Tue 22 Jan 10:58 PST 2019, Stephen Boyd wrote: > > > Quoting Bjorn Andersson (2019-01-21 21:51:03) > > > @@ -103,10 +138,30 @@ > > > no-map; > > > }; > > > > > > + venus_mem: memory@95800000 { > > > + reg = <0 0x95800000 0 0x500000>; > > > + no-map; > > > + }; > > > + > > > + cdsp_mem: memory@95d00000 { > > > + reg = <0 0x95d00000 0 0x800000>; > > > + no-map; > > > + }; > > > + > > > mba_region: memory@96500000 { > > > reg = <0 0x96500000 0 0x200000>; > > > no-map; > > > }; > > > + > > > + slpi_mem: memory@96700000 { > > > + reg = <0 0x96700000 0 0x1400000>; > > > + no-map; > > > + }; > > > + > > > + spss_mem: memory@97b00000 { > > > + reg = <0 0x97b00000 0 0x100000>; > > > + no-map; > > > + }; > > > }; > > > > > > > What's the plan if certain configurations don't use all these carveouts? > > Can we mark the reservation nodes as status = "disabled", or the reverse > > and mark them as status = "ok" in all boards, and then reclaim the > > memory for peripherals we don't care to use? > > > > The code path that picks these up does look for "status", so I suggest > that we leave them all enabled in the platform dtsi and then let the > device's reclaim them as needed. Does that mean we should add labels for all of the sub-nodes so that boards can easily mark them "disabled"? -Doug
Hi, On Mon, Jan 21, 2019 at 9:52 PM Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > Update existing and add all missing PIL regions to the reserved memory > map, as described in version 10. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > > Changes since v2: > - New patch > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 61 ++++++++++++++++++++++++++-- > 1 file changed, 58 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index 0ec827394e92..cdcac3704c13 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -89,12 +89,47 @@ > }; > > memory@86200000 { > - reg = <0 0x86200000 0 0x2d00000>; > + reg = <0 0x86200000 0 0x100000>; > no-map; > }; > > - wlan_msa_mem: memory@96700000 { > - reg = <0 0x96700000 0 0x100000>; > + memory@86300000 { > + reg = <0 0x86300000 0 0x4800000>; > + no-map; > + }; I know it's not a problem upstream (yet), but downstream this collides with a memory region in the cheza board. We have: rmtfs@88f00000 { compatible = "qcom,rmtfs-mem"; reg = <0x0 0x88f00000 0x0 0x800000>; no-map; qcom,client-id = <1>; }; ...and the above region overlays it since it goes till 0x8ab00000 > + > + memory@8ab00000 { > + reg = <0 0x8ab00000 0 0x1400000>; > + no-map; > + }; > + > + memory@8bf00000 { > + reg = <0 0x8bf00000 0 0x500000>; > + no-map; > + }; > + > + ipa_fw_mem: memory@8c400000 { > + reg = <0 0x8c400000 0 0x10000>; > + no-map; > + }; > + > + ipa_gsi_mem: memory@8c410000 { > + reg = <0 0x8c410000 0 0x5000>; > + no-map; > + }; > + > + memory@8c415000 { > + reg = <0 0x8c415000 0 0x2000>; > + no-map; > + }; > + > + adsp_mem: memory@8c500000 { > + reg = <0 0x8c500000 0 0x1a00000>; > + no-map; > + }; > + > + wlan_msa_mem: memory@8df00000 { Your patch moves 'wlan_msa_mem' from 0x96700000 to 0x8df00000. Is that OK? I haven't been involved in all of the previous discussions but if everything is all OK w/ the device tree just moving this chunk around (without any other coordination w/ firmware) it seems really weird that we even need to specify it in the device tree. ...but maybe I shouldn't open this can of worms. You can pretend I didn't say anything. -Doug
On Tue 22 Jan 15:10 PST 2019, Doug Anderson wrote: > Hi, > > On Tue, Jan 22, 2019 at 11:24 AM Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > > > On Tue 22 Jan 10:58 PST 2019, Stephen Boyd wrote: > > > > > Quoting Bjorn Andersson (2019-01-21 21:51:03) > > > > @@ -103,10 +138,30 @@ > > > > no-map; > > > > }; > > > > > > > > + venus_mem: memory@95800000 { > > > > + reg = <0 0x95800000 0 0x500000>; > > > > + no-map; > > > > + }; > > > > + > > > > + cdsp_mem: memory@95d00000 { > > > > + reg = <0 0x95d00000 0 0x800000>; > > > > + no-map; > > > > + }; > > > > + > > > > mba_region: memory@96500000 { > > > > reg = <0 0x96500000 0 0x200000>; > > > > no-map; > > > > }; > > > > + > > > > + slpi_mem: memory@96700000 { > > > > + reg = <0 0x96700000 0 0x1400000>; > > > > + no-map; > > > > + }; > > > > + > > > > + spss_mem: memory@97b00000 { > > > > + reg = <0 0x97b00000 0 0x100000>; > > > > + no-map; > > > > + }; > > > > }; > > > > > > > > > > What's the plan if certain configurations don't use all these carveouts? > > > Can we mark the reservation nodes as status = "disabled", or the reverse > > > and mark them as status = "ok" in all boards, and then reclaim the > > > memory for peripherals we don't care to use? > > > > > > > The code path that picks these up does look for "status", so I suggest > > that we leave them all enabled in the platform dtsi and then let the > > device's reclaim them as needed. > > Does that mean we should add labels for all of the sub-nodes so that > boards can easily mark them "disabled"? > That sounds reasonable, I'll dig up some labels for the unlabeled nodes as well. Thanks, Bjorn
On Tue 22 Jan 15:16 PST 2019, Doug Anderson wrote: > Hi, > > On Mon, Jan 21, 2019 at 9:52 PM Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > > > Update existing and add all missing PIL regions to the reserved memory > > map, as described in version 10. > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > --- > > > > Changes since v2: > > - New patch > > > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 61 ++++++++++++++++++++++++++-- > > 1 file changed, 58 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > index 0ec827394e92..cdcac3704c13 100644 > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > @@ -89,12 +89,47 @@ > > }; > > > > memory@86200000 { > > - reg = <0 0x86200000 0 0x2d00000>; > > + reg = <0 0x86200000 0 0x100000>; > > no-map; > > }; > > > > - wlan_msa_mem: memory@96700000 { > > - reg = <0 0x96700000 0 0x100000>; > > + memory@86300000 { > > + reg = <0 0x86300000 0 0x4800000>; > > + no-map; > > + }; > > I know it's not a problem upstream (yet), but downstream this collides > with a memory region in the cheza board. We have: > > rmtfs@88f00000 { > compatible = "qcom,rmtfs-mem"; > reg = <0x0 0x88f00000 0x0 0x800000>; > no-map; > > qcom,client-id = <1>; > }; > > ...and the above region overlays it since it goes till 0x8ab00000 > Digging through the table again I see that there's another level here, so it seems only the first 44MB of these 78MB are reserved for non-APSS things. So this should actually be 0x2c00000 long. I will update this and we'll have one conflict less. > > > + > > + memory@8ab00000 { > > + reg = <0 0x8ab00000 0 0x1400000>; > > + no-map; > > + }; > > + > > + memory@8bf00000 { > > + reg = <0 0x8bf00000 0 0x500000>; > > + no-map; > > + }; > > + > > + ipa_fw_mem: memory@8c400000 { > > + reg = <0 0x8c400000 0 0x10000>; > > + no-map; > > + }; > > + > > + ipa_gsi_mem: memory@8c410000 { > > + reg = <0 0x8c410000 0 0x5000>; > > + no-map; > > + }; > > + > > + memory@8c415000 { > > + reg = <0 0x8c415000 0 0x2000>; > > + no-map; > > + }; > > + > > + adsp_mem: memory@8c500000 { > > + reg = <0 0x8c500000 0 0x1a00000>; > > + no-map; > > + }; > > + > > + wlan_msa_mem: memory@8df00000 { > > Your patch moves 'wlan_msa_mem' from 0x96700000 to 0x8df00000. Is > that OK? I haven't been involved in all of the previous discussions > but if everything is all OK w/ the device tree just moving this chunk > around (without any other coordination w/ firmware) it seems really > weird that we even need to specify it in the device tree. ...but > maybe I shouldn't open this can of worms. You can pretend I didn't > say anything. > 0x96700000 seems to be reserved for the sensor core, so either WiFi wasn't actually tested before, or more likely its firmware is position independent. Most (all?) firmware is position independent, but the security configuration prevents us from relocating it. One such example is that the ADSP in the newer firmware versions are not allowed to execute from the old memory region. Regards, Bjorn
Hey Bjorn, On 2019-01-22 11:21, Bjorn Andersson wrote: > Update existing and add all missing PIL regions to the reserved memory > map, as described in version 10. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > > Changes since v2: > - New patch > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 61 ++++++++++++++++++++++++++-- > 1 file changed, 58 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi > b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index 0ec827394e92..cdcac3704c13 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -89,12 +89,47 @@ > }; Shouldn't we add hyp and xbl regions as well? > > memory@86200000 { > - reg = <0 0x86200000 0 0x2d00000>; > + reg = <0 0x86200000 0 0x100000>; > no-map; > }; > > - wlan_msa_mem: memory@96700000 { > - reg = <0 0x96700000 0 0x100000>; > + memory@86300000 { > + reg = <0 0x86300000 0 0x4800000>; from v10 I see we don't need to reserve the last 28M it just needs to be reg = <0 0x86300000 0 0x2c00000>; > + no-map; > + }; > + > + memory@8ab00000 { > + reg = <0 0x8ab00000 0 0x1400000>; > + no-map; > + }; > + > + memory@8bf00000 { > + reg = <0 0x8bf00000 0 0x500000>; > + no-map; > + }; > + > + ipa_fw_mem: memory@8c400000 { > + reg = <0 0x8c400000 0 0x10000>; > + no-map; > + }; > + > + ipa_gsi_mem: memory@8c410000 { > + reg = <0 0x8c410000 0 0x5000>; > + no-map; > + }; > + > + memory@8c415000 { > + reg = <0 0x8c415000 0 0x2000>; > + no-map; > + }; > + > + adsp_mem: memory@8c500000 { > + reg = <0 0x8c500000 0 0x1a00000>; > + no-map; > + }; > + > + wlan_msa_mem: memory@8df00000 { > + reg = <0 0x8df00000 0 0x100000>; > no-map; > }; > > @@ -103,10 +138,30 @@ > no-map; > }; > > + venus_mem: memory@95800000 { > + reg = <0 0x95800000 0 0x500000>; > + no-map; > + }; > + > + cdsp_mem: memory@95d00000 { > + reg = <0 0x95d00000 0 0x800000>; > + no-map; > + }; > + > mba_region: memory@96500000 { should we re-name mba_region/mpss_region to mba_mem and mpss_mem for consistency. > reg = <0 0x96500000 0 0x200000>; > no-map; > }; > + > + slpi_mem: memory@96700000 { > + reg = <0 0x96700000 0 0x1400000>; > + no-map; > + }; > + > + spss_mem: memory@97b00000 { > + reg = <0 0x97b00000 0 0x100000>; > + no-map; > + }; > }; > > cpus {
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index 0ec827394e92..cdcac3704c13 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -89,12 +89,47 @@ }; memory@86200000 { - reg = <0 0x86200000 0 0x2d00000>; + reg = <0 0x86200000 0 0x100000>; no-map; }; - wlan_msa_mem: memory@96700000 { - reg = <0 0x96700000 0 0x100000>; + memory@86300000 { + reg = <0 0x86300000 0 0x4800000>; + no-map; + }; + + memory@8ab00000 { + reg = <0 0x8ab00000 0 0x1400000>; + no-map; + }; + + memory@8bf00000 { + reg = <0 0x8bf00000 0 0x500000>; + no-map; + }; + + ipa_fw_mem: memory@8c400000 { + reg = <0 0x8c400000 0 0x10000>; + no-map; + }; + + ipa_gsi_mem: memory@8c410000 { + reg = <0 0x8c410000 0 0x5000>; + no-map; + }; + + memory@8c415000 { + reg = <0 0x8c415000 0 0x2000>; + no-map; + }; + + adsp_mem: memory@8c500000 { + reg = <0 0x8c500000 0 0x1a00000>; + no-map; + }; + + wlan_msa_mem: memory@8df00000 { + reg = <0 0x8df00000 0 0x100000>; no-map; }; @@ -103,10 +138,30 @@ no-map; }; + venus_mem: memory@95800000 { + reg = <0 0x95800000 0 0x500000>; + no-map; + }; + + cdsp_mem: memory@95d00000 { + reg = <0 0x95d00000 0 0x800000>; + no-map; + }; + mba_region: memory@96500000 { reg = <0 0x96500000 0 0x200000>; no-map; }; + + slpi_mem: memory@96700000 { + reg = <0 0x96700000 0 0x1400000>; + no-map; + }; + + spss_mem: memory@97b00000 { + reg = <0 0x97b00000 0 0x100000>; + no-map; + }; }; cpus {
Update existing and add all missing PIL regions to the reserved memory map, as described in version 10. Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- Changes since v2: - New patch arch/arm64/boot/dts/qcom/sdm845.dtsi | 61 ++++++++++++++++++++++++++-- 1 file changed, 58 insertions(+), 3 deletions(-)