Hi, On 8/22/22 15:02, Andy Shevchenko wrote: > On Mon, Aug 22, 2022 at 12:50 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: >> >> alloc_user_pages() is only ever called on qbuf for USERPTR buffers which >> always hits the get_user_pages_fast() path, so the pin_user_pages() path >> can be removed. >> >> Getting the vma then also is no longer necessary since that is only >> done to determine which path to use. >> >> And this also removes the only users of the mem_type struct hmm_bo member, >> so remove that as well. > > ... > >> + /*Handle frame buffer allocated in user space*/ > > Spaces? I just changed the indentation on this, but I might just as well add the spaces while at it, will do so for v2. > >> + mutex_unlock(&bo->mutex); > >> + page_nr = get_user_pages_fast((unsigned long)userptr, >> + (int)(bo->pgnr), 1, bo->pages); > > No need for parentheses in the first argument. Ack, will fix for v2. > >> + mutex_lock(&bo->mutex); > >> + dev_dbg(atomisp_dev, "%s: %d user pages were allocated as 0x%08x\n", >> + __func__, bo->pgnr, page_nr); > > Since you touch this you may remove __func__, which can be enabled via > dynamic debug. OTOH, it might be better to go and drop __func__ > everywhere in the driver in the debug messages. Or even better, I'll just drop this useless debug statement entirely for v2. Regards, Hans