On Thu, Oct 17, 2024 at 11:02:33AM +0300, Stanimir Varbanov wrote: > On 10/16/24 22:38, Bjorn Helgaas wrote: > > On Wed, Oct 16, 2024 at 01:09:00PM -0400, Jim Quinlan wrote: > >> On Mon, Oct 14, 2024 at 1:25 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > >>> On Mon, Oct 14, 2024 at 10:10:11AM -0700, Florian Fainelli wrote: > >>>> On 10/14/24 09:57, Bjorn Helgaas wrote: > >>>>> On Mon, Oct 14, 2024 at 04:07:03PM +0300, Stanimir Varbanov wrote: > >>>>>> BCM2712 memory map can supports up to 64GB of system > >>>>>> memory, thus expand the inbound size calculation in > >>>>>> helper function up to 64GB. > >>>>> > >>>>> The fact that the calculation is done in a helper isn't important > >>>>> here. Can you make the subject line say something about supporting > >>>>> DMA for up to 64GB of system memory? > >>>>> > >>>>> This is being done specifically for BCM2712, but I assume it's safe > >>>>> for *all* brcmstb devices, right? > >>>> > >>>> It is safe in the sense that all brcmstb devices with this PCIe > >>>> controller will adopt the same encoding of the size, all of the > >>>> currently supported brcmstb devices have a variety of > >>>> limitations when it comes to the amount of addressable DRAM > >>>> however. Typically we have a hard limit at 4GB of DRAM per > >>>> memory controller, some devices can do 2GB x3, 4GB x2, or 4GB > >>>> x1. > >>>> > >>>> Does that answer your question? > >>> > >>> I'd like something in the commit log to the effect that while > >>> we're doing this to support more system memory on BCM2712, this > >>> change is safe for other SoCs that don't support as much system > >>> memory. > >> > >> This setting configures the size of an RC's inbound window to system > >> memory. Any inbound access outside of all of the inbound windows > >> will be discarded. > >> > >> Some existing SoCs cannot support the 64GB size. Configuring such > >> an SoC to 64GB will effectively disable the entire window. > > > > So I *think* you're saying that this patch will break existing SoCs > > that don't support the 64GB size, right? > > Existing SoCs will not be impacted. It could be theoretically possible > to break inbound window translations only if you wrongly populate window > sizes in DT. I guess this is the part that I missed -- the inbound window sizes come from DT (via bridge->dma_ranges, IIUC), and the patch merely supports encoding of larger windows than previously. Bjorn