Re: [PATCH v1] dma-mapping: Fix filename references

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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@linux.intel.com> 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