Re: [PATCH v10 2/5] of: Stop DMA translation at last DMA parent

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 8, 2022 at 8:33 AM Thierry Reding <thierry.reding@xxxxxxxxx> wrote:
>
> On Mon, Nov 07, 2022 at 01:30:35PM -0600, Rob Herring wrote:
> > On Thu, Nov 03, 2022 at 02:38:57PM +0100, Thierry Reding wrote:
> > > From: Thierry Reding <treding@xxxxxxxxxx>
> > >
> > > DMA parent devices can define separate DMA busses via the "dma-ranges"
> > > and "#address-cells" and "#size-cells" properties. If the DMA bus has
> > > different cell counts than its parent, this can cause the translation
> > > of DMA address to fails (e.g. truncation from 2 to 1 address cells).
> >
> > My assumption in this case was that the parent cell sizes should be
> > increased to 2 cells. That tends to be what people want to do anyways
> > (64-bit everywhere on 64-bit CPUs).
> >
> > > Avoid this by stopping to search for DMA parents when a parent without
> > > a "dma-ranges" property is encountered. Also, since it is the DMA parent
> > > that defines the DMA bus, use the bus' cell counts instead of its parent
> > > cell counts.
> >
> > We treat no 'dma-ranges' as equivalent to 'dma-ranges;'. IIRC, the spec
> > even says that because I hit that case.
> >
> > Is this going to work for 'dma-device' with something like this?:
> >
> >   bus@0 {
> >     dma-ranges = <...>;
> >     child-bus@... {
> >       dma-device@... {
> >       };
> >     };
> >   };
> >
> > >
> > > Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> > > ---
> > > Changes in v10:
> > > - new patch to avoid address truncation when traversing a bus hierarchy
> > >   with mismatching #address-cells properties
> > >
> > > Example from Tegra194 (redacted for clarity):
> > >
> > >     reserved-memory {
> > >             #address-cells = <2>;
> > >             #size-cells = <2>;
> > >             ranges;
> > >
> > >             framebuffer@0,0 {
> > >                     compatible = "framebuffer";
> > >                     reg = <0x2 0x57320000 0x0 0x00800000>;
> > >                     iommu-addresses = <&dc0 0x2 0x57320000 0x0 0x00800000>;
> > >             };
> > >     };
> > >
> > >     bus@0 {
> > >             /* truncation happens here */
> > >             #address-cells = <1>;
> > >             #size-cells = <1>;
> > >             ranges = <0x0 0x0 0x0 0x40000000>;
> > >
> > >             mc: memory-controller@2c00000 {
> > >                     #address-cells = <2>;
> > >                     #size-cells = <2>;
> >
> > I think this is wrong. The parent should have more or equal number of
> > cells.
>
> I was half suspecting that. The reason why I hesitated is that I recall
> having the opposite discussion a while ago when we were adding bus@0 to
> 64-bit Tegra devices. We had at some point (probably around Tegra114 or
> Tegra124, 32-bit ARM chips that support LPAE) started to set #address-
> cells = <2> precisely because the CPU could address more than 32-bit
> addresses. We then did the same thing transitioning to 64-bit ARM. When
> we then started discussing bus@0, someone (might have been you) had
> argued that all these peripherals could be addressed with a single cell
> so there'd be no need for #address-cells = <2>, so then we went with
> that.

I may have not thinking about the DMA side of things.

> Reverting back to #address-cells = <2> is now going to cause quite a bit
> of churn, but I guess if it's the right thing, so be it.
>
> Another possible alternative would be to move the memory-controller node
> from the bus@0 to the top-level. Not sure if that's any better.

I stumbled upon 'ibm,#dma-address-cells' and 'ibm,#dma-size-cells'
while reviewing this. Those seem to be for the same purpose AFAICT. We
could consider adding those (w/o 'ibm') to handle this situation.

Rob



[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux