Hi Andy, On Thu, Jun 4, 2020 at 11:05 AM Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > On Thu, Jun 04, 2020 at 10:35:12AM -0400, Jim Quinlan wrote: > > On Thu, Jun 4, 2020 at 9:53 AM Nicolas Saenz Julienne > > <nsaenzjulienne@xxxxxxx> wrote: > > > On Wed, 2020-06-03 at 15:20 -0400, Jim Quinlan wrote: > > ... > > > > > + phys = virt_to_phys(ret); > > > > + pfn = phys >> PAGE_SHIFT; > > > > > > nit: not sure it really pays off to have a pfn variable here. > > Did it for readability; the compiler's optimization should take care > > of any extra variables. But I can switch if you insist. > > One side note: please, try to get familiar with existing helpers in the kernel. > For example, above line is like > > pfn = PFN_DOWN(phys); I just used the term in the original code; will change to PFN_DOWN(). > > ... > > > > > + if (!WARN_ON(!dev) && dev->dma_pfn_offset_map) > > > > > + *dma_handle -= PFN_PHYS( > > > > + dma_pfn_offset_from_phys_addr(dev, phys)); > > Don't do such indentation, esp. we have now 100! :-) Got it. Thanks, Jim Quinlan > > -- > With Best Regards, > Andy Shevchenko > > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel