Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 4 Aug 2020 12:22:23 -0700
From:   Bjorn Andersson <bjorn.andersson@linaro.org>
To:     Amit Pundir <amit.pundir@linaro.org>
Cc:     Andy Gross <agross@kernel.org>, Rob Herring <robh+dt@kernel.org>,
        John Stultz <john.stultz@linaro.org>,
        Sumit Semwal <sumit.semwal@linaro.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        dt <devicetree@vger.kernel.org>,
        lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3] arm64: dts: qcom: Add support for Xiaomi Poco F1
 (Beryllium)
Message-ID: <20200804192223.GD20825@builder.lan>
References: <1596297341-13549-1-git-send-email-amit.pundir@linaro.org>
 <20200804061652.GK61202@yoga>
 <CAMi1Hd1KCmoF7D06otBcxCBYEUW3Tdn_aYQ722=yRoEeZYmTPg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAMi1Hd1KCmoF7D06otBcxCBYEUW3Tdn_aYQ722=yRoEeZYmTPg@mail.gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue 04 Aug 00:35 PDT 2020, Amit Pundir wrote:
> On Tue, 4 Aug 2020 at 11:46, Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
> > On Sat 01 Aug 08:55 PDT 2020, Amit Pundir wrote:
[..]
> > > diff --git a/arch/arm64/boot/dts/qcom/sdm845-beryllium.dts b/arch/arm64/boot/dts/qcom/sdm845-beryllium.dts
[..]
> > > +/* Reserved memory changes from downstream */
> > > +/ {
> > > +     reserved-memory {
> > > +             removed_region: memory@88f00000 {
> >
> > Do you know what these 26MB are used for? Do you think it's possible to
> > give it a more appropriate label? The size happens to be the same as
> > &adsp_mem from sdm845.dtsi, this is probably not a coincidence.
> 
> In downstream, this removed region is marked as removed-dma-pool
> compatible, https://github.com/MiCode/Xiaomi_Kernel_OpenSource/commit/b982b6dc77ac34c184abe83dd293ac08fc607ba3,
> a carved out memory region not exposed to the kernel. I honestly
> didn't know what to name this region, so I kept it as it was. I can't
> boot past the bootloader if I don't mark this region as reserved.
> 

That seems to imply that it's used for something other than ADSP, but
the size is strange.

The removed-dma-pool is a downstream construct, the important part is
no-map;

> >
> > That said, this overlaps at least &rmtfs_mem, &qseecom_mem and
> > &camera_mem, so I would expect that you have a few warnings about this
> > early in the log? Please shuffle things around to avoid this.
> 
> Sorry, I do know that it definitely overlaps with upstream &rmtfs_mem,
> but I ignored that because plan was to just boot to shell with this
> base dts. I was planning to submit a follow-up patch with downstream
> reserved memory mappings, which do not coincide with the upstream
> sdm845.dtsi mem regions you mentioned above, along with relevant
> wifi/adsp/cdsp nodes. But let me take this opportunity to submit them
> in the next version of this base dts.
> 

This problem does come up from time to time and I have a similar problem
with getting the IPA driver to probe on my SDM850 laptop, because the
sdm845.dtsi reserved-memory regions isn't accepted.

So I think we should move (most of?) the reserved-memory regions from
sdm845.dtsi into the individual device dts files - given that it
obviously differs between different devices.


I'm okay with your plan, but please add a TODO comment here describing
that this is needed to boot the device, that you know it is overlapping
with other regions and that the memory map needs more work for this
device.

Thanks,
Bjorn