Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220113170755.11856-1-pali@kernel.org> <CAK8P3a2D8Yv+KpM4NJyP9mosieqbhHh08=mdEy+OA84Vx6FVCQ@mail.gmail.com>
 <20220218165530.j62nafuofe342sfi@pali>
In-Reply-To: <20220218165530.j62nafuofe342sfi@pali>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Fri, 18 Feb 2022 18:19:57 +0100
Message-ID: <CAK8P3a1CHRztMCOFN6iomVf2J7_9qRi4GdZBQfDyhjqDb1Z0Vw@mail.gmail.com>
Subject: Re: [PATCH] arm64: dts: marvell: armada-37xx: Increase PCIe IO size
 from 64 KiB to 1 MiB
To:     =?UTF-8?Q?Pali_Roh=C3=A1r?= <pali@kernel.org>
Cc:     Arnd Bergmann <arnd@arndb.de>,
        Gregory Clement <gregory.clement@bootlin.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        =?UTF-8?B?TWFyZWsgQmVow7pu?= <kabel@kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        DTML <devicetree@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

On Fri, Feb 18, 2022 at 5:55 PM Pali Rohár <pali@kernel.org> wrote:
> On Friday 18 February 2022 17:43:04 Arnd Bergmann wrote:
> > On Thu, Jan 13, 2022 at 6:07 PM Pali Rohár <pali@kernel.org> wrote:
> > >
> > > Commit 514ef1e62d65 ("arm64: dts: marvell: armada-37xx: Extend PCIe MEM
> > > space") increased size of PCIe MEM to 127 MiB, which is the maximal
> > > possible size for allocated 128 MiB PCIe window. PCIe IO size in that
> > > commit was unchanged.
> > >
> > > Armada 3720 PCIe controller supports 32-bit IO space mapping so it is
> > > possible to assign more than 64 KiB if address space for IO.
> > >
> > > Currently controller has assigned 127 MiB + 64 KiB memory and therefore
> > > there is 960 KiB of unused memory. So assign it to IO space by increasing
> > > IO window from 64 KiB to 1 MiB.
> > >
> > > DTS file armada-3720-turris-mox.dts already uses whole 128 MiB space, so
> > > only update comment about 32-bit IO space mapping.
> > >
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > Fixes: 514ef1e62d65 ("arm64: dts: marvell: armada-37xx: Extend PCIe MEM space")
> >
> > I just saw this is the fixes pull request, and it seems very odd. Does this
> > fix an actual bug?
>
> Do you mean this patch or commit 514ef1e62d65?

This one. 514ef1e62d65 looks fine.

> > Note that Linux normally doesn't map more than 64KB
> > of I/O space per PCI domain, so it should not make a difference to us.
>
> Last time I looked into ARM code, it can allocate more than 64 kB for IO.


> > Also, note that having a high bus address for the I/O space (0xefff0000,
> > as as the CPU physical address here) means that a lot of the older
> > devices that actually require I/O space won't work, because they need a
> > low bus address in the first few KB.
> >
> > Is this mapping a requirement from a broken bootloader, or can you change
> > the mapping of the I/O port window in the physical space to the usual
> > bus address 0?
>
> At physical address 0x0 it is not possible as at this address is mapped
> DDR.

I meant bus address 0, not CPU physical address 0 of course. We don't
care where in physical space the I/O window is.

> ARM Trusted-Firmware sets PCIe space to range [0xe8000000-0xf0000000].
> This (default) configuration is specified in DTS file. Which parts of
> this range is used for IO and which MEM is up to the a3720 PCIe kernel
> driver and currently it configures it based on how sub-ranges are
> specified in DT.
>
> In some cases (e.g. when board has 4 GB of RAM), TF-A relocates this
> PCIe range to different location (otherwise it cannot activate more than
> 2 GB of RAM) and U-Boot during loading of kernel DTB file, is patching
> it.
>
> It could be possible to change TF-A code to move PCIe space to different
> location (from [0xe8000000-0xf0000000]) but not to 0x0. But changing it
> means to move other parts and invent mapping in which most of RAM can be
> mapped to...

Can't you change the mapping to have a bus address that is different
the physical address?

       Arnd