On 10/18/19 2:53 PM, Robin Murphy wrote: > On 18/10/2019 13:22, Marek Vasut wrote: >> On 10/18/19 11:53 AM, Lorenzo Pieralisi wrote: >>> On Thu, Oct 17, 2019 at 05:01:26PM +0200, Marek Vasut wrote: >>> >>> [...] >>> >>>>> Again, just handling the first N dma-ranges entries and ignoring the >>>>> rest is not 'configure the controller correctly'. >>>> >>>> It's the best effort thing to do. It's well possible the next >>>> generation >>>> of the controller will have more windows, so could accommodate the >>>> whole >>>> list of ranges. > > In the context of DT describing the platform that doesn't make any > sense. It's like saying it's fine for U-Boot to also describe a bunch of > non-existent CPUs just because future SoCs might have them. Just because > the system would probably still boot doesn't mean it's right. It's the exact opposite of what you just described -- the last release of U-Boot currently populates a subset of the DMA ranges, not a superset. The dma-ranges in the Linux DT currently are a superset of available DRAM on the platform. >>>> Thinking about this further, this patch should be OK either way, if >>>> there is a DT which defines more DMA ranges than the controller can >>>> handle, handling some is better than failing outright -- a PCI which >>>> works with a subset of memory is better than PCI that does not work >>>> at all. >>> >>> OK to sum it up, this patch is there to deal with u-boot adding multiple >>> dma-ranges to DT. >> >> Yes, this patch was posted over two months ago, about the same time this >> functionality was posted for inclusion in U-Boot. It made it into recent >> U-Boot release, but there was no feedback on the Linux patch until >> recently. >> >> U-Boot can be changed for the next release, assuming we agree on how it >> should behave. >> >>> I still do not understand the benefit given that for >>> DMA masks they are useless as Rob pointed out and ditto for inbound >>> windows programming (given that AFAICS the PCI controller filters out >>> any transaction that does not fall within its inbound windows by default >>> so adding dma-ranges has the net effect of widening the DMA'able address >>> space rather than limiting it). >>> >>> In short, what's the benefit of adding more dma-ranges regions to the >>> DT (and consequently handling them in the kernel) ? >> >> The benefit is programming the controller inbound windows correctly. >> But if there is a better way to do that, I am open to implement that. >> Are there any suggestions / examples of that ? > > The crucial thing is that once we improve the existing "dma-ranges" > handling in the DMA layer such that it *does* consider multiple entries > properly, platforms presenting ranges which don't actually exist will > almost certainly start going wrong, and are either going to have to fix > their broken bootloaders or try to make a case for platform-specific > workarounds in core code. Again, this is exactly the other way around, the dma-ranges populated by U-Boot cover only existing DRAM. The single dma-range in Linux DT covers even the holes without existing DRAM. So even if the Linux dma-ranges handling changes, there should be no problem. [...] -- Best regards, Marek Vasut
