Re: [PATCH v2 6/7] misc: bcm-vk: add Broadcom VK driver

From: Dan Carpenter
Date: Sat Apr 18 2020 - 07:48:00 EST


On Sat, Apr 18, 2020 at 02:45:16PM +0300, Dan Carpenter wrote:
> On Fri, Apr 17, 2020 at 02:49:11PM -0700, Scott Branden wrote:
> > > > +static int bcm_vk_dma_alloc(struct device *dev,
> > > > + struct bcm_vk_dma *dma,
> > > > + int direction,
> > > > + struct _vk_data *vkdata)
> > > > +{
> > > > + dma_addr_t addr, sg_addr;
> > > > + int err;
> > > > + int i;
> > > > + int offset;
> > > > + uint32_t size;
> > > > + uint32_t remaining_size;
> > > > + uint32_t transfer_size;
> > > > + uint64_t data;
> > > > + unsigned long first, last;
> > > > + struct _vk_data *sgdata;
> > > > +
> > > > + /* Get 64-bit user address */
> > > > + data = get_unaligned(&(vkdata->address));
> > > Extra parens.
> > removed
> > >
> > > > +
> > > > + /* offset into first page */
> > > > + offset = offset_in_page(data);
> > > > +
> > > > + /* Calculate number of pages */
> > > > + first = (data & PAGE_MASK) >> PAGE_SHIFT;
> > > > + last = ((data + vkdata->size - 1) & PAGE_MASK) >> PAGE_SHIFT;
> > > > + dma->nr_pages = last - first + 1;
> > > > +
> > > > + /* Allocate DMA pages */
> > > > + dma->pages = kmalloc_array(dma->nr_pages,
> > > > + sizeof(struct page *),
> > > > + GFP_KERNEL);
> > > > + if (dma->pages == NULL)
> > > > + return -ENOMEM;
> > > > +
> > > > + dev_dbg(dev, "Alloc DMA Pages [0x%llx+0x%x => %d pages]\n",
> > > > + data, vkdata->size, dma->nr_pages);
> > > > +
> > > > + dma->direction = direction;
> > > > +
> > > > + /* Get user pages into memory */
> > > > + err = get_user_pages_fast(data & PAGE_MASK,
> > > > + dma->nr_pages,
> > > > + direction == DMA_FROM_DEVICE,
> > > > + dma->pages);
> > > > + if (err != dma->nr_pages) {
> > > > + dma->nr_pages = (err >= 0) ? err : 0;
> > > > + dev_err(dev, "get_user_pages_fast, err=%d [%d]\n",
> > > > + err, dma->nr_pages);
> > > > + return err < 0 ? err : -EINVAL;
> > > > + }
> > > > +
> > > > + /* Max size of sg list is 1 per mapped page + fields at start */
> > > > + dma->sglen = (dma->nr_pages * sizeof(*sgdata)) +
> > > > + (sizeof(uint32_t) * SGLIST_VKDATA_START);
> > > > +
> > > > + /* Allocate sglist */
> > > > + dma->sglist = dma_alloc_coherent(dev,
> > > > + dma->sglen,
> > > > + &dma->handle,
> > > > + GFP_KERNEL);
> > >
> > > dma->sglist = dma_alloc_coherent(dev, dma->sglen, &dma->handle,
> > > GFP_KERNEL);
> > done
> > >
> > >
> > >
> > > > + if (!dma->sglist)
> > > > + return -ENOMEM;
> > > No cleanup?
> > what needs to be cleaned up?
>
> dma->pages should be freed probably? And a put_user_pages_fast()?

Sorry put_user_pages_fast() isn't a function. My bad.

regards,
dan carpenter