On Wed, Mar 20, 2019 at 04:31:17PM -0500, Bjorn Helgaas wrote:
> On Wed, Mar 20, 2019 at 11:13 AM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> >
> > After the commit cf65a0f6f6ff
> >
> > ("dma-mapping: move all DMA mapping code to kernel/dma")
> >
> > some of the files are referring to outdated information, i.e. old file names
> > of DMA mapping sources.
> >
> > Fix it here.
Bjorn, thanks for review, my answers below.
> > * This function checks if the reserved crashkernel is allowed on the specific
> > * IA64 machine flavour. Machines without an IO TLB use swiotlb and require
> > * some memory below 4 GB (i.e. in 32 bit area), see the implementation of
> > - * lib/swiotlb.c. The hpzx1 architecture has an IO TLB but cannot use that
> > + * kernel/dma/swiotlb.c. The hpzx1 architecture has an IO TLB but cannot use that
> > * in kdump case. See the comment in sba_init() in sba_iommu.c.
>
> Is the point here that just that if you lack an IOTLB and want devices
> to be able to reach system memory above 4GB, you need a bounce buffer
> below 4GB? If so, maybe we could just say *that* instead of a
> nebulous reference to "the implementation of */swiotlb.c", which
> doesn't tell you what part of the implementation is relevant.
This patch is about broken links.
> > --- a/arch/x86/kernel/pci-swiotlb.c
> > +++ b/arch/x86/kernel/pci-swiotlb.c
> > @@ -1,5 +1,5 @@
> > // SPDX-License-Identifier: GPL-2.0
> > -/* Glue code to lib/swiotlb.c */
> > +/* Glue code to kernel/dma/swiotlb.c */
>
> I personally don't think there's much value in this line and I'd
> remove it entirely.
Will do.
> > /*
> > - * two parts from lib/swiotlb.c:
> > + * two parts from kernel/dma/swiotlb.c:
> > * -swiotlb size: user-specified with swiotlb= or default.
> > *
> > * -swiotlb overflow buffer: now hardcoded to 32k. We round it
>
> Is there any chance this could be updated to refer to some variable or
> function names that grep/ctags/cscope/etc could find? If we had that,
> I think the filename reference would be superfluous.
I'm not sure. Mauro probably the best person to ask what Sphinx can do here.
> > /*
> > * arch/x86/pci/sta2x11-fixup.c
> > - * glue code for lib/swiotlb.c and DMA translation between STA2x11
> > + * glue code for kernel/dma/swiotlb.c and DMA translation between STA2x11
>
> I think both of these lines (the "arch/x86/pci/sta2x11-fixup.c" one
> and the "glue code" one) are superfluous.
Will remove.
> There's also a superfluous empty line at the end of the GPL license
> text, but I guess that should be removed by replacing the whole thing
> with an SPDX line.
Correct.
--
With Best Regards,
Andy Shevchenko
